From 2fb54e05091e8d20d02cc96df8756cf447327ad8 Mon Sep 17 00:00:00 2001 From: Alissa Renz Date: Tue, 22 Oct 2024 19:16:38 -0700 Subject: [PATCH 1/4] Add metadata to Assistant say util [WIP] --- src/Assistant.ts | 19 +++++++++++++------ test/unit/Assistant.spec.ts | 29 +++++++++++++++++++---------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/Assistant.ts b/src/Assistant.ts index da84f5cbd..aec9a1dec 100644 --- a/src/Assistant.ts +++ b/src/Assistant.ts @@ -3,6 +3,7 @@ import type { AssistantThreadsSetSuggestedPromptsResponse, AssistantThreadsSetTitleResponse, ChatPostMessageArguments, + MessageMetadataEventPayloadObject, } from '@slack/web-api'; import { type AssistantThreadContext, @@ -132,7 +133,7 @@ export class Assistant { private async processEvent(args: AllAssistantMiddlewareArgs): Promise { const { payload } = args; - const assistantArgs = enrichAssistantArgs(this.threadContextStore, args); + const assistantArgs = await enrichAssistantArgs(this.threadContextStore, args); const assistantMiddleware = this.getAssistantMiddleware(payload); return processAssistantMiddleware(assistantArgs, assistantMiddleware); } @@ -160,10 +161,10 @@ export class Assistant { * events from continuing down the global middleware chain to subsequent listeners * 2. Adds assistant-specific utilities (i.e., helper methods) * */ -export function enrichAssistantArgs( +export async function enrichAssistantArgs( threadContextStore: AssistantThreadContextStore, args: AllAssistantMiddlewareArgs, // TODO: the type here states that these args already have the assistant utilities present? the type here needs likely changing. -): AllAssistantMiddlewareArgs { +): Promise { const { next: _next, ...assistantArgs } = args; const preparedArgs = { ...(assistantArgs as Exclude, 'next'>) }; @@ -171,7 +172,7 @@ export function enrichAssistantArgs( preparedArgs.getThreadContext = () => threadContextStore.get(args); preparedArgs.saveThreadContext = () => threadContextStore.save(args); - preparedArgs.say = createSay(preparedArgs); + preparedArgs.say = await createSay(preparedArgs); preparedArgs.setStatus = createSetStatus(preparedArgs); preparedArgs.setSuggestedPrompts = createSetSuggestedPrompts(preparedArgs); preparedArgs.setTitle = createSetTitle(preparedArgs); @@ -299,14 +300,20 @@ export async function processAssistantMiddleware( * was received. Alias for `postMessage()`. * https://api.slack.com/methods/chat.postMessage */ -function createSay(args: AllAssistantMiddlewareArgs): SayFn { +async function createSay(args: AllAssistantMiddlewareArgs): Promise { const { client, payload } = args; - const { channelId: channel, threadTs: thread_ts } = extractThreadInfo(payload); + const { channelId: channel, threadTs: thread_ts, context } = extractThreadInfo(payload); + const threadContext = context.channel_id ? context : await args.getThreadContext(args); return (message: Parameters[0]) => { const postMessageArgument: ChatPostMessageArguments = typeof message === 'string' ? { text: message, channel, thread_ts } : { ...message, channel, thread_ts }; + postMessageArgument.metadata = { + event_type: 'assistant_thread_context', + event_payload: threadContext as MessageMetadataEventPayloadObject, + }; + return client.chat.postMessage(postMessageArgument); }; } diff --git a/test/unit/Assistant.spec.ts b/test/unit/Assistant.spec.ts index bbd52f8f6..fe66ade89 100644 --- a/test/unit/Assistant.spec.ts +++ b/test/unit/Assistant.spec.ts @@ -194,9 +194,12 @@ describe('Assistant class', () => { const { enrichAssistantArgs } = await importAssistant(); - const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); - const threadContextChangedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadContextChangedArgs); - const userMessageArgs = enrichAssistantArgs(mockThreadContextStore, mockUserMessageArgs); + const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + const threadContextChangedArgs = await enrichAssistantArgs( + mockThreadContextStore, + mockThreadContextChangedArgs, + ); + const userMessageArgs = await enrichAssistantArgs(mockThreadContextStore, mockUserMessageArgs); assert.notExists(threadStartedArgs.next); assert.notExists(threadContextChangedArgs.next); @@ -208,7 +211,9 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); // TODO: enrichAssistantArgs likely needs a different argument type, as AssistantMiddlewareArgs type already has the assistant utility enrichments present. - const assistantArgs = enrichAssistantArgs(mockThreadContextStore, { payload } as AllAssistantMiddlewareArgs); + const assistantArgs = await enrichAssistantArgs(mockThreadContextStore, { + payload, + } as AllAssistantMiddlewareArgs); assert.exists(assistantArgs.say); assert.exists(assistantArgs.setStatus); @@ -221,7 +226,9 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); // TODO: enrichAssistantArgs likely needs a different argument type, as AssistantMiddlewareArgs type already has the assistant utility enrichments present. - const assistantArgs = enrichAssistantArgs(mockThreadContextStore, { payload } as AllAssistantMiddlewareArgs); + const assistantArgs = await enrichAssistantArgs(mockThreadContextStore, { + payload, + } as AllAssistantMiddlewareArgs); assert.exists(assistantArgs.say); assert.exists(assistantArgs.setStatus); @@ -234,7 +241,9 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); // TODO: enrichAssistantArgs likely needs a different argument type, as AssistantMiddlewareArgs type already has the assistant utility enrichments present. - const assistantArgs = enrichAssistantArgs(mockThreadContextStore, { payload } as AllAssistantMiddlewareArgs); + const assistantArgs = await enrichAssistantArgs(mockThreadContextStore, { + payload, + } as AllAssistantMiddlewareArgs); assert.exists(assistantArgs.say); assert.exists(assistantArgs.setStatus); @@ -299,7 +308,7 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); - const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); await threadStartedArgs.say('Say called!'); @@ -314,7 +323,7 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); - const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); await threadStartedArgs.setStatus('Status set!'); @@ -329,7 +338,7 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); - const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); await threadStartedArgs.setSuggestedPrompts({ prompts: [{ title: '', message: '' }] }); @@ -344,7 +353,7 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); - const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); await threadStartedArgs.setTitle('Title set!'); From 79a3331c03fcc02086cf524cfd0aa4bae629f012 Mon Sep 17 00:00:00 2001 From: Alissa Renz Date: Wed, 23 Oct 2024 17:35:42 -0700 Subject: [PATCH 2/4] Add tests --- test/unit/Assistant.spec.ts | 47 +++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/unit/Assistant.spec.ts b/test/unit/Assistant.spec.ts index fe66ade89..7d58e949a 100644 --- a/test/unit/Assistant.spec.ts +++ b/test/unit/Assistant.spec.ts @@ -315,6 +315,53 @@ describe('Assistant class', () => { sinon.assert.called(fakeClient.chat.postMessage); }); + it('say should be called with message_metadata that includes thread context', async () => { + const mockThreadStartedArgs = wrapMiddleware(createDummyAssistantThreadStartedEventMiddlewareArgs()); + + const fakeClient = { chat: { postMessage: sinon.spy() } }; + mockThreadStartedArgs.client = fakeClient as unknown as WebClient; + const mockThreadContextStore = createMockThreadContextStore(); + + const { enrichAssistantArgs } = await importAssistant(); + const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + + await threadStartedArgs.say('Say called!'); + + const { + payload: { + assistant_thread: { channel_id, thread_ts, context }, + }, + } = mockThreadStartedArgs; + + const expectedParams = { + text: 'Say called!', + channel: channel_id, + thread_ts, + metadata: { + event_type: 'assistant_thread_context', + event_payload: context, + }, + }; + + sinon.assert.calledWith(fakeClient.chat.postMessage, expectedParams); + }); + + it('say should get context from store if no thread context is included in event', async () => { + const mockThreadStartedArgs = wrapMiddleware(createDummyAssistantThreadStartedEventMiddlewareArgs()); + mockThreadStartedArgs.payload.assistant_thread.context = {}; + + const fakeClient = { chat: { postMessage: sinon.spy() } }; + mockThreadStartedArgs.client = fakeClient as unknown as WebClient; + const mockThreadContextStore = { save: sinon.spy(), get: sinon.spy() }; + + const { enrichAssistantArgs } = await importAssistant(); + const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + + await threadStartedArgs.say('Say called!'); + + sinon.assert.calledOnce(mockThreadContextStore.get); + }); + it('setStatus should call assistant.threads.setStatus', async () => { const mockThreadStartedArgs = wrapMiddleware(createDummyAssistantThreadStartedEventMiddlewareArgs()); From 661df5426e3474ebd8370d5ef6e24dea326a1d80 Mon Sep 17 00:00:00 2001 From: Alissa Renz Date: Thu, 24 Oct 2024 17:31:19 -0700 Subject: [PATCH 3/4] say util metadata JIT --- src/Assistant.ts | 24 +++++++++++++----------- test/unit/Assistant.spec.ts | 27 ++++++++++++--------------- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/Assistant.ts b/src/Assistant.ts index aec9a1dec..ddcc44c27 100644 --- a/src/Assistant.ts +++ b/src/Assistant.ts @@ -133,7 +133,7 @@ export class Assistant { private async processEvent(args: AllAssistantMiddlewareArgs): Promise { const { payload } = args; - const assistantArgs = await enrichAssistantArgs(this.threadContextStore, args); + const assistantArgs = enrichAssistantArgs(this.threadContextStore, args); const assistantMiddleware = this.getAssistantMiddleware(payload); return processAssistantMiddleware(assistantArgs, assistantMiddleware); } @@ -161,10 +161,10 @@ export class Assistant { * events from continuing down the global middleware chain to subsequent listeners * 2. Adds assistant-specific utilities (i.e., helper methods) * */ -export async function enrichAssistantArgs( +export function enrichAssistantArgs( threadContextStore: AssistantThreadContextStore, args: AllAssistantMiddlewareArgs, // TODO: the type here states that these args already have the assistant utilities present? the type here needs likely changing. -): Promise { +): AllAssistantMiddlewareArgs { const { next: _next, ...assistantArgs } = args; const preparedArgs = { ...(assistantArgs as Exclude, 'next'>) }; @@ -172,7 +172,7 @@ export async function enrichAssistantArgs( preparedArgs.getThreadContext = () => threadContextStore.get(args); preparedArgs.saveThreadContext = () => threadContextStore.save(args); - preparedArgs.say = await createSay(preparedArgs); + preparedArgs.say = createSay(preparedArgs); preparedArgs.setStatus = createSetStatus(preparedArgs); preparedArgs.setSuggestedPrompts = createSetSuggestedPrompts(preparedArgs); preparedArgs.setTitle = createSetTitle(preparedArgs); @@ -300,19 +300,21 @@ export async function processAssistantMiddleware( * was received. Alias for `postMessage()`. * https://api.slack.com/methods/chat.postMessage */ -async function createSay(args: AllAssistantMiddlewareArgs): Promise { +function createSay(args: AllAssistantMiddlewareArgs): SayFn { const { client, payload } = args; const { channelId: channel, threadTs: thread_ts, context } = extractThreadInfo(payload); - const threadContext = context.channel_id ? context : await args.getThreadContext(args); - return (message: Parameters[0]) => { + return async (message: Parameters[0]) => { + const threadContext = context.channel_id ? context : await args.getThreadContext(args); const postMessageArgument: ChatPostMessageArguments = typeof message === 'string' ? { text: message, channel, thread_ts } : { ...message, channel, thread_ts }; - postMessageArgument.metadata = { - event_type: 'assistant_thread_context', - event_payload: threadContext as MessageMetadataEventPayloadObject, - }; + if (threadContext) { + postMessageArgument.metadata = { + event_type: 'assistant_thread_context', + event_payload: threadContext as MessageMetadataEventPayloadObject, + }; + } return client.chat.postMessage(postMessageArgument); }; diff --git a/test/unit/Assistant.spec.ts b/test/unit/Assistant.spec.ts index 7d58e949a..ddf22e8cf 100644 --- a/test/unit/Assistant.spec.ts +++ b/test/unit/Assistant.spec.ts @@ -194,12 +194,9 @@ describe('Assistant class', () => { const { enrichAssistantArgs } = await importAssistant(); - const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); - const threadContextChangedArgs = await enrichAssistantArgs( - mockThreadContextStore, - mockThreadContextChangedArgs, - ); - const userMessageArgs = await enrichAssistantArgs(mockThreadContextStore, mockUserMessageArgs); + const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + const threadContextChangedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadContextChangedArgs); + const userMessageArgs = enrichAssistantArgs(mockThreadContextStore, mockUserMessageArgs); assert.notExists(threadStartedArgs.next); assert.notExists(threadContextChangedArgs.next); @@ -211,7 +208,7 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); // TODO: enrichAssistantArgs likely needs a different argument type, as AssistantMiddlewareArgs type already has the assistant utility enrichments present. - const assistantArgs = await enrichAssistantArgs(mockThreadContextStore, { + const assistantArgs = enrichAssistantArgs(mockThreadContextStore, { payload, } as AllAssistantMiddlewareArgs); @@ -226,7 +223,7 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); // TODO: enrichAssistantArgs likely needs a different argument type, as AssistantMiddlewareArgs type already has the assistant utility enrichments present. - const assistantArgs = await enrichAssistantArgs(mockThreadContextStore, { + const assistantArgs = enrichAssistantArgs(mockThreadContextStore, { payload, } as AllAssistantMiddlewareArgs); @@ -241,7 +238,7 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); // TODO: enrichAssistantArgs likely needs a different argument type, as AssistantMiddlewareArgs type already has the assistant utility enrichments present. - const assistantArgs = await enrichAssistantArgs(mockThreadContextStore, { + const assistantArgs = enrichAssistantArgs(mockThreadContextStore, { payload, } as AllAssistantMiddlewareArgs); @@ -308,7 +305,7 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); - const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); await threadStartedArgs.say('Say called!'); @@ -323,7 +320,7 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); - const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); await threadStartedArgs.say('Say called!'); @@ -355,7 +352,7 @@ describe('Assistant class', () => { const mockThreadContextStore = { save: sinon.spy(), get: sinon.spy() }; const { enrichAssistantArgs } = await importAssistant(); - const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); await threadStartedArgs.say('Say called!'); @@ -370,7 +367,7 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); - const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); await threadStartedArgs.setStatus('Status set!'); @@ -385,7 +382,7 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); - const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); await threadStartedArgs.setSuggestedPrompts({ prompts: [{ title: '', message: '' }] }); @@ -400,7 +397,7 @@ describe('Assistant class', () => { const mockThreadContextStore = createMockThreadContextStore(); const { enrichAssistantArgs } = await importAssistant(); - const threadStartedArgs = await enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); await threadStartedArgs.setTitle('Title set!'); From e85abb9bf5ca1db78ca2cc5c4be22f30c15227cd Mon Sep 17 00:00:00 2001 From: Alissa Renz Date: Fri, 25 Oct 2024 14:19:24 -0700 Subject: [PATCH 4/4] Update test --- test/unit/Assistant.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/unit/Assistant.spec.ts b/test/unit/Assistant.spec.ts index ddf22e8cf..61071f832 100644 --- a/test/unit/Assistant.spec.ts +++ b/test/unit/Assistant.spec.ts @@ -354,6 +354,9 @@ describe('Assistant class', () => { const { enrichAssistantArgs } = await importAssistant(); const threadStartedArgs = enrichAssistantArgs(mockThreadContextStore, mockThreadStartedArgs); + // Verify that get is not called prior to say being used + sinon.assert.notCalled(mockThreadContextStore.get); + await threadStartedArgs.say('Say called!'); sinon.assert.calledOnce(mockThreadContextStore.get);