-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(api-service): adopt Chat SDK emoji system for cross-platform agent reactions #10764
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
Changes from all commits
92a2d63
71abe96
643d84d
193d0db
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,12 @@ | ||
| import { BadRequestException, forwardRef, Inject, Injectable, OnModuleDestroy } from '@nestjs/common'; | ||
| import { PinoLogger } from '@novu/application-generic'; | ||
| import type { Chat, Message, Thread } from 'chat'; | ||
| import type { Chat, EmojiValue, Message, ReactionEvent, Thread } from 'chat'; | ||
| import { Request as ExpressRequest, Response as ExpressResponse } from 'express'; | ||
| import { LRUCache } from 'lru-cache'; | ||
| import { AgentEventEnum } from '../dtos/agent-event.enum'; | ||
| import { AgentPlatformEnum } from '../dtos/agent-platform.enum'; | ||
| import type { ReplyContentDto } from '../dtos/agent-reply-payload.dto'; | ||
| import { esmImport } from '../utils/esm-import'; | ||
| import { sendWebResponse, toWebRequest } from '../utils/express-to-web-request'; | ||
| import { AgentConfigResolver, ResolvedAgentConfig } from './agent-config-resolver.service'; | ||
| import { AgentInboundHandler } from './agent-inbound-handler.service'; | ||
|
|
@@ -26,11 +27,6 @@ import { AgentInboundHandler } from './agent-inbound-handler.service'; | |
| * credentials.phoneNumberIdentification → phoneNumberId | ||
| */ | ||
|
|
||
| // Chat SDK packages are ESM-only; SWC rewrites import() → require() for CJS output. | ||
| // Wrapping in new Function prevents SWC from seeing the import() keyword. | ||
| // eslint-disable-next-line @typescript-eslint/no-implied-eval | ||
| const esmImport = new Function('specifier', 'return import(specifier)') as (s: string) => Promise<any>; | ||
|
|
||
| const MAX_CACHED_INSTANCES = 200; | ||
| const INSTANCE_TTL_MS = 1000 * 60 * 30; | ||
|
|
||
|
|
@@ -133,14 +129,15 @@ export class ChatSdkService implements OnModuleDestroy { | |
| platform: string, | ||
| platformThreadId: string, | ||
| platformMessageId: string, | ||
| emoji: string | ||
| emojiName: string | ||
| ): Promise<void> { | ||
| const config = await this.agentConfigResolver.resolve(agentId, integrationIdentifier); | ||
| const instanceKey = `${agentId}:${integrationIdentifier}`; | ||
| const chat = await this.getOrCreate(instanceKey, agentId, config.platform, config); | ||
|
|
||
| const adapter = chat.getAdapter(platform); | ||
| await adapter.removeReaction(platformThreadId, platformMessageId, emoji); | ||
| const resolved = await this.resolveEmoji(emojiName); | ||
| await adapter.removeReaction(platformThreadId, platformMessageId, resolved); | ||
| } | ||
|
|
||
| async reactToMessage( | ||
|
|
@@ -149,14 +146,25 @@ export class ChatSdkService implements OnModuleDestroy { | |
| platform: string, | ||
| platformThreadId: string, | ||
| platformMessageId: string, | ||
| emoji: string | ||
| emojiName: string | ||
| ): Promise<void> { | ||
| const config = await this.agentConfigResolver.resolve(agentId, integrationIdentifier); | ||
| const instanceKey = `${agentId}:${integrationIdentifier}`; | ||
| const chat = await this.getOrCreate(instanceKey, agentId, config.platform, config); | ||
|
|
||
| const adapter = chat.getAdapter(platform); | ||
| await adapter.addReaction(platformThreadId, platformMessageId, emoji); | ||
| const resolved = await this.resolveEmoji(emojiName); | ||
| await adapter.addReaction(platformThreadId, platformMessageId, resolved); | ||
| } | ||
|
|
||
| private async resolveEmoji(name: string): Promise<EmojiValue> { | ||
| const { getEmoji } = await esmImport('chat'); | ||
| const resolved = getEmoji(name); | ||
| if (!resolved) { | ||
| throw new Error(`Unknown emoji name: "${name}". Use GET /agents/emoji to list supported options.`); | ||
| } | ||
|
|
||
| return resolved; | ||
| } | ||
|
Comment on lines
+160
to
168
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check whether esmImport memoizes its result (so per-call overhead is a no-op after first load).
fd -t f 'esm-import.ts' | xargs -I{} sh -c 'echo "=== {} ==="; cat "{}"'Repository: novuhq/novu Length of output: 440 🏁 Script executed: # Check if BadRequestException is imported in the file
grep -n "BadRequestException\|import.*Exception" apps/api/src/app/agents/services/chat-sdk.service.ts | head -20Repository: novuhq/novu Length of output: 633 🏁 Script executed: # Find resolveEmoji call sites to understand frequency and context
rg "resolveEmoji" apps/api/src/app/agents/services/ -A 2 -B 2Repository: novuhq/novu Length of output: 1330 🏁 Script executed: # Look for the `@IsWellKnownEmoji` validator definition
fd -t f . apps/api | xargs rg "@IsWellKnownEmoji|IsWellKnownEmoji" -lRepository: novuhq/novu Length of output: 173 🏁 Script executed: # Check the entire chat-sdk.service.ts file to understand the structure and caching patterns
wc -l apps/api/src/app/agents/services/chat-sdk.service.tsRepository: novuhq/novu Length of output: 111 🏁 Script executed: # Search for any existing emoji module caching at module level
rg "chat.*emoji|emoji.*chat" apps/api/src/app/agents/services/chat-sdk.service.ts -B 2 -A 2Repository: novuhq/novu Length of output: 37 🏁 Script executed: # Check the `@IsWellKnownEmoji` validator implementation
cat -n apps/api/src/app/agents/validators/is-well-known-emoji.validator.tsRepository: novuhq/novu Length of output: 1718 🏁 Script executed: # Look at the removeReaction and addReaction methods to understand the full flow
rg "removeReaction|addReaction" apps/api/src/app/agents/services/chat-sdk.service.ts -B 5 -A 5Repository: novuhq/novu Length of output: 1206 🏁 Script executed: # Check for DTO usage to see where `@IsWellKnownEmoji` is applied
rg "@IsWellKnownEmoji" apps/api/src/app/agents/dtos/ -B 2 -A 2Repository: novuhq/novu Length of output: 801 Use Two confirmed issues in
Suggested fix for error handling- private async resolveEmoji(name: string): Promise<EmojiValue> {
- const { getEmoji } = await esmImport('chat');
- const resolved = getEmoji(name);
- if (!resolved) {
- throw new Error(`Unknown emoji name: "${name}". Use GET /agents/emoji to list supported options.`);
- }
-
- return resolved;
- }
+ private async resolveEmoji(name: string): Promise<EmojiValue> {
+ const { getEmoji } = await esmImport('chat');
+ const resolved = getEmoji(name);
+ if (!resolved) {
+ throw new BadRequestException(
+ `Unknown emoji name: "${name}". Use GET /agents/emoji to list supported options.`
+ );
+ }
+
+ return resolved;
+ }🤖 Prompt for AI Agents |
||
|
|
||
| private async getOrCreate( | ||
|
|
@@ -379,14 +387,14 @@ export class ChatSdkService implements OnModuleDestroy { | |
| } | ||
| }); | ||
|
|
||
| cached.chat.onReaction(async (event: any) => { | ||
| cached.chat.onReaction(async (event: ReactionEvent) => { | ||
| try { | ||
| await this.inboundHandler.handleReaction(agentId, cached.config, { | ||
| emoji: event.emoji, | ||
| added: event.added, | ||
| messageId: event.messageId, | ||
| message: event.message, | ||
| thread: event.thread, | ||
| thread: event.thread as Thread | undefined, | ||
| user: event.user, | ||
| }); | ||
| } catch (err) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Method name and missing
@ApiResponsedecorator.Two guideline deviations:
listEmojidoesn't match thelist{EntityName}pattern used elsewhere in this controller (listAgents,listAgentIntegrations). Rename tolistAgentEmojifor consistency.@ApiResponsedecorator. Even though the controller is@ApiExcludeController, every other endpoint here declares one and it documents the response shape for generated clients/dashboards.Proposed change
As per coding guidelines: "Controller method names follow the pattern:
getEntityName,listEntityName, ..." and "Every endpoint must have@ApiOperation,@ApiResponse, and@ApiTagsdecorators for OpenAPI documentation".🤖 Prompt for AI Agents