-
Notifications
You must be signed in to change notification settings - Fork 395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: auto acknowledge assistant events #2330
base: autoack
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## autoack #2330 +/- ##
===========================================
- Coverage 91.08% 91.04% -0.05%
===========================================
Files 22 22
Lines 6116 6140 +24
Branches 655 666 +11
===========================================
+ Hits 5571 5590 +19
- Misses 540 545 +5
Partials 5 5 ☔ View full report in Codecov by Sentry. |
I'm going on record (again) to say that I really am not a fan of this approach. The If my assumption above is incorrect and the delayed If accurate, then it would be better IMO to introduce an option (similar to the opt-out of JIT function tokens) that is specific and limited to function listeners receiving the ability to |
If we were to follow the JIT token opt-out approach, this would result in auto-ack being a global option, applying to all custom functions or none. I don't like this suggestion as it would force developers who have existing custom functions that answer to I'm not married to any particular implementation, but I do feel like the auto-ack feature should be implemented in a way so as to enable per-function granularity. If the middleware based approach is unacceptable, then let's revert it and go back to the drawing board. |
I've introduced some changes that fix the bugs brought by #2283 to the assistant handlers. These changes were taken from the behavior of Bolt-Python and aim to align the python and js architectures |
Summary
MEGRE #2329 FIRST!
Since #2283 event requests are no longer auto acknowledged in the processEvent function. This behavior was handed over to the autoAcknowledge middleware. This PR aims to add auto acknowledge behavior back into the assistant event handlers.
Requirements