Add support for Agents/Assistants

[misscoded]

Summary

Adds support for Agents/Assistants with the introduction of an Assistant class, making it easy to get up and running with a Slack Agent/Assistant.

Example Usage

const assistant = new Assistant({
  threadContextStore: {
    get: async ({ context, client, payload }) => {},
    save: async ({ context, client, payload }) => {},
  },
  threadStarted: async ({ say, saveThreadContext, setStatus, setSuggestedPrompts, setTitle }) => {},
  threadContextChanged: async ({ say, setStatus, setSuggestedPrompts, setTitle }) => {},
  userMessage: async ({ say, getThreadContext, setStatus, setSuggestedPrompts, setTitle }) => {},
});

app.assistant(assistant);

AssistantConfig

threadContextStore: (Optional, but Recommended) A custom AssistantThreadContextStore can be provided by the developer, so long as it's inclusive of the required methods to get and save thread context. When provided, these methods will override the getThreadContext and saveThreadContext utilities that are made available in other Assistant event listeners. If not provided, a DefaultAssistantContextStore is used, though this is not intended for production purposes (relies on updating message metadata in the container's thread).

threadStarted: (Required) Executes when the assistant_thread_started event is received. Sent when a user opens the Assistant container or otherwise begins a new chat. This can happen via DM with the app or as a side-container within a channel.

threadContextChanged: (Optional) Executes when the assistant_thread_context_changed event is received. Sent when a user switches channels while the Assistant container is open. If threadContextChanged is not provided, context will be saved using the AssistantContextStore's save method (either the DefaultAssistantContextStore or custom, if provided).

userMessage: (Required) Executes when a message is sent to the Assistant. These messages do not contain a subtype and must be deduced based on their shape and metadata (if provided). Bolt handles this deduction out of the box for those using the Assistant class.

Assistant Utilities

getThreadContext: Alias for AssistantContextStore.get(). If custom AssistantContextStore is provided, the get method will be executed. If not provided, the DefaultAssistantContextStore will find and retrieve the most recent context that has either been saved to the instance or attached to the initial Assistant message that was sent to the thread.

saveThreadContext: Alias for AssistantContextStore.save(). If custom AssistantContextStore is provided, the save method will be executed. If not provided, the DefaultAssistantContextStore will extract the assistant_thread.context from the event and save it to the instance and attach it to the initial Assistant message that was sent to the thread.

say(message: string): Alias for postMessage. Sends a message to the current Assistant thread.

setTitle(title: string): Set the title of the Assistant thread to capture the initial topic/question as a way to facilitate future reference by the user. Learn more: https://api.slack.com/methods/assistant.threads.setTitle

setStatus(status: string): Set the status of the Assistant to give the appearance of active processing. Learn more: https://api.slack.com/methods/assistant.threads.setStatus

setSuggestedPrompts({ prompts: [{ title: string; message: string; }]: Provide the user up to 4 optional, preset prompts to choose from. Learn more: https://api.slack.com/methods/assistant.threads.setSuggestedPrompts

Requirements (place an x in each [ ])

• I've read and understood the Contributing Guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.

[codecov]

Codecov Report

Attention: Patch coverage is 77.12418% with 35 lines in your changes missing coverage. Please review.

Project coverage is 81.21%. Comparing base (70debca) to head (ad7a190).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/Assistant.ts	75.63%	20 Missing and 9 partials ⚠️
src/App.ts	0.00%	3 Missing ⚠️
src/AssistantThreadContextStore.ts	88.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2286      +/-   ##
==========================================
- Coverage   81.59%   81.21%   -0.38%     
==========================================
  Files          19       21       +2     
  Lines        1646     1799     +153     
  Branches      464      498      +34     
==========================================
+ Hits         1343     1461     +118     
- Misses        194      218      +24     
- Partials      109      120      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[seratch]

Great work! I've not done any thorough runtime tests tho!

[seratch]

Just a comment for future maintainers; no need to change anything -- Allowing multiple listeners may cause confusion (e.g., unnecessarily calling the threadContextStore several times) and almost 100% developers need only one listener here. With that being said, perhaps we still should keep the consistency in bolt-js (while bolt-python/java allow only one listener by design).

[misscoded]

Good point. Adding more context: with this architecture and where it's been used with the two other features, we have never acted on behalf of the developer: what they provide is what we run. We deviate from this in this feature by automatically calling save on their behalf and behind the scenes, if certain conditions are not met.

[seratch]

This pull request does not support Block Kit interactions + botMessage listener. For initial release, that's fine, but we may receive feature requests to add the advanced feature, which is available in bolt-python and bolt-java

[misscoded]

Can you point me to what you're referring to, specifically within the Python implementation? Without looking at the code, I presume you mean when a user interacts with a BKE, that is also directed to this listener (rather than the other middleware that exists for it)?

[seratch]

You can check examples/assistants/interaction_app.py to see how it works

[misscoded]

I had a look, but all I see in that example appears to be a hand-rolled approach that combines the manual adding, parsing, and handling of metadata paired with the use of regular listeners.

What about the current implementation would prevent that same approach from being used? What is not being handled by Bolt behind the scenes and on behalf of the developer that should?

[seratch]

Your current implementation does not provide a way to handle the app's bot message events. Something equivalent to ignoring_self_assistant_message_events_enabled=False is necessary if a developer would like to post a message with user inputs first, and then continue its processing in a following listener (botMessage listener). It's still feasible to do eveything within app.view listener but the user experience is not optimal plus the code can be less simple.

[misscoded]

I've thought about this a bit longer, and if it's OK with you, I'd like to avoid introducing more conveniences until we know how folks are actually using this. If @filmaj finishes his review and disagrees, I'll introduce it, but I'm concerned that we're already making this quite complicated/opinionated out of the box before anyone has used it or requested additional features.

[filmaj]

A fast follow seems reasonable to me 🤷 ; also I have a large PR (bolt v4 #2254) that is waiting on this to be merged, so I selfishly would like to prevent adding more until I can reconcile these two (which will require a bit of work AFAICT)

[seratch]

This is definitely an advanced usage of the feature, so I don't think it's a release blocker for the initial rollout. Also, nothing prevents us from adding the feature later on. We can add it in future releases without any issues.

[filmaj]

Silly question before I dive in to code review: is the thread context store required? It's unclear to me the benefit it provides. I must be missing context (🥁)... related, I think we'll need to add a new doc for this feature too, right?

[misscoded]

Silly question before I dive in to code review: is the thread context store required? It's unclear to me the benefit it provides. I must be missing context (🥁)... related, I think we'll need to add a new doc for this feature too, right?

Not a silly question at all, @filmaj. @seratch led the design here, and this is what his recommendation was, based on his experience using this feature (and what was implemented in Python and Java). FWIW, I also don't think it's technically necessary: developers could get just as far with us demonstrating how it should be done via documentation and templates/samples. It's more of a convenience.

I defer to @seratch, as the lead of this design, but I'm OK with whatever is decided.

[filmaj]

Left a first round of comments, mostly a bunch of questions (to help inform me how to reconcile this PR w/ the bolt-v4 branch).

One general point is I am hesitant to support exporting so many helper functions from Assistant.ts - devs will leverage those even if we tell them not to (unless that is the intention). Many of these are only used in one place or solely within Assistant.ts. In those cases, can we remove the export? Or if the function is only used in one place, remove it altogether and just use it in the one place it is used? I find that helps reduce cognitive load trying to understand the code, especially for short functions (I don't have to e.g. search the codebase to see where this function is used to understand what the impact is to changing it - this is particularly helpful later when making TypeScript changes, which I will have to do to fit this into bolt v4). Essentially the functions defined after the Assistant class in Assistant.ts.

Last point is we probably need docs to go with this on https://tools.slack.dev/bolt-js/, probably a new doc under "Basic Concepts"?

[filmaj]

May I ask we rename this from prepare to enrich? Anything named "prepare" is a pet peeve of mine as the definition for what "preparation" means requires knowing the context. We name other such middleware-argument-enriching processes elsewhere (like in the deno-sdk) enriching. 🙏

Also, AFAICT this function only gets used once, so let's not export it - less API surface area to possibly break in the future. Could even just move it to where it is used (inside processEvent here).

[misscoded]

I export these only for testing purposes! Sounds like that's an outdated approach? If it's possible to test these without exporting, that'll allow me to get rid of the export.

And sure, a very reluctant OK on the rename. 😛

[filmaj]

oic, no tests yet so you are not importing them yet, is that it? My IDE was telling me they were only used in one spot / one file for most of them like prepareAssistantArgs, isAssistantEvent, matchesConstraint, isAssistantMessage, and a couple others in that area.

[misscoded]

Nope, prepareAssistantArgs is definitely used in tests - it has its own block! I imagine the others are there, too. They're destructured from a parent import.

[filmaj]

Ah thanks for pointing that out. OK, nevermind 😞 The async imports must have confused my IDE search functionality.

[misscoded]

No worries. I'm doing a pass just to make sure we're not exporting anything unnecessarily for good measure.

[seratch]

@filmaj

is the thread context store required? It's unclear to me the benefit it provides. I must be missing context (🥁)... related, I think we'll need to add a new doc for this feature too, right?

I've published draft document PRs for python/java and am waiting for Alissa's reviews. If you can take a look at them too, it'd be appreciated: https://github.com/slackapi/bolt-python/pull/1175/files

The reason the context store is important is that handling context change events is typically just boilerplate. Since the changed context data is not provided in subsequent user message events due to server-side design, you (the bolt app) need to remember the latest context. Without this context store mechanism, developers would have to create it from scratch, and the result would largely replicate this built-in context store. With the current design, developers have two options: 1) implement their own context change event handler (in this case, the built-in context store won't be used), or 2) customize only the method for fetching/saving updated context data, meaning you implement your own context store and pass it to the assistant middleware.

Let me know if this still does not make sense to you

[filmaj]

LGTM. Re: docs I'll leave that up to you; since docs get deployed on-demand, could be another fast-follow, too. Your call.

[filmaj]

the changed context data is not provided in subsequent user message events due to server-side design

Thanks @seratch , that was the missing piece. Makes sense: message events have their own pre-assistant design. And thanks for linking to the WIP docs PR, will review shortly, and knowing this information about context store will also help in my documentation review.