feat(api-service): adopt Chat SDK emoji system for cross-platform agent reactions#10764
feat(api-service): adopt Chat SDK emoji system for cross-platform agent reactions#10764
Conversation
|
Hey there and thank you for opening this pull request! 👋 We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted. Your PR title is: Requirements:
Expected format: Details: PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new GET /agents/emoji endpoint and ListAgentEmoji use case, introduces runtime-validated emoji names and async emoji resolution/loading, updates types to WellKnownEmoji, adapts chat reaction handling to resolve emoji names, and updates the dashboard to fetch and render emojis dynamically. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Dashboard / API consumer)
participant Controller as AgentsController
participant UseCase as ListAgentEmoji
participant Chat as chat (DEFAULT_EMOJI_MAP)
Client->>Controller: GET /agents/emoji
Controller->>UseCase: execute()
UseCase->>Chat: esmImport('chat') / read DEFAULT_EMOJI_MAP
Chat-->>UseCase: DEFAULT_EMOJI_MAP
UseCase-->>Controller: AgentEmojiEntry[] (cached on first call)
Controller-->>Client: 200 OK (AgentEmojiEntry[])
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (3)
apps/dashboard/src/components/agents/teams-app-package.ts (1)
22-22: Consider adding a fallback forroundRectif browser support matrix expands to older versions.
roundRectis widely supported in modern browsers (Chrome 99+, Safari 16+, Firefox 112+) with 98%+ global coverage as of 2026. Given the project targets ES2020 without explicit legacy browser configuration, this should not cause issues in practice. However, for defensive programming, consider adding a polyfill or fallback path toarc()if you ever need to support older browsers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/agents/teams-app-package.ts` at line 22, The call to ctx.roundRect may fail in older browsers; add a runtime fallback that checks for ctx.roundRect and if absent draws the rounded rectangle manually using ctx.beginPath, ctx.moveTo/lineTo and ctx.arc (using the same size and corner radius currently passed), then closePath and fill/stroke as appropriate; update the block that currently calls ctx.roundRect(0, 0, size, size, size * 0.18) to use this feature-detecting path so ctx.roundRect is used when present and the manual arc-based drawing is used otherwise.apps/api/src/app/agents/services/chat-sdk.service.ts (1)
126-126: Consider narrowingemojiNametoWellKnownEmoji.The PR goal is to make emoji values type-safe across the codebase, but
reactToMessage/removeReactionstill acceptstring. SinceresolveEmojiwill fail for unknown names (viagetEmoji), narrowing the parameter at the API boundary pushes the check to compile time for internal callers (e.g.,HandleAgentReply).Proposed change
-import type { Chat, EmojiValue, Message, ReactionEvent, Thread } from 'chat'; +import type { Chat, EmojiValue, Message, ReactionEvent, Thread, WellKnownEmoji } from 'chat'; @@ - emojiName: string + emojiName: WellKnownEmoji ): Promise<void> { @@ - emojiName: string + emojiName: WellKnownEmoji ): Promise<void> {Also applies to: 143-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/services/chat-sdk.service.ts` at line 126, Change the emojiName type from string to the WellKnownEmoji union at the public API boundary so callers get compile-time safety: update the emojiName property/parameter in ChatSdkService methods reactToMessage and removeReaction (and any other occurrences at the same spots) to WellKnownEmoji, import or re-export the WellKnownEmoji type where needed, and adjust any callers that pass arbitrary strings to use a WellKnownEmoji value (or explicitly cast/validate before calling) so resolveEmoji/getEmoji no longer receives unchecked strings.apps/api/src/app/agents/dtos/agent-behavior.dto.ts (1)
16-16: ImportWellKnownEmojifrom'chat'and use it to type the reaction fields.The validator decorators already enforce
WellKnownEmojiat runtime, but the TypeScript property types remainstring | null. Aligning the types with the validator will enable compile-time narrowing for callers and align with the pattern already used inagent-config-resolver.service.ts.+import type { WellKnownEmoji } from 'chat'; import { IsWellKnownEmoji } from '../validators/is-well-known-emoji.validator'; export class AgentReactionSettingsDto { `@ApiPropertyOptional`({ description: 'Cross-platform emoji name for incoming messages (e.g. "eyes", "thumbs_up"). ' + 'Set to null to disable. Default: "eyes"', default: 'eyes', }) `@IsOptional`() `@ValidateIf`((_, value) => value !== null) `@IsWellKnownEmoji`() - onMessageReceived?: string | null; + onMessageReceived?: WellKnownEmoji | null; `@ApiPropertyOptional`({ description: 'Cross-platform emoji name for resolved conversations (e.g. "check", "star"). ' + 'Set to null to disable. Default: "check"', default: 'check', }) `@IsOptional`() `@ValidateIf`((_, value) => value !== null) `@IsWellKnownEmoji`() - onResolved?: string | null; + onResolved?: WellKnownEmoji | null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/dtos/agent-behavior.dto.ts` at line 16, Import WellKnownEmoji from 'chat' and use it as the TypeScript type for the reaction fields instead of string | null (e.g., change the onMessageReceived property's type to WellKnownEmoji | null); apply the same change to any other emoji/reaction properties in the agent-behavior.dto.ts DTO so the compile-time types match the runtime validators; ensure the new import is added at the top of the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/agents/agents.controller.ts`:
- Around line 85-95: Rename the controller method listEmoji to listAgentEmoji
for consistency, update its call to use listAgentEmojiUsecase.execute(), add an
explicit return type (e.g., Promise<AgentEmojiDto[]> or the existing DTO type
used elsewhere), and annotate the method with an `@ApiResponse`(...) decorator
describing the 200 response schema and type; keep the existing `@ApiOperation` and
`@RequirePermissions`(PermissionsEnum.AGENT_READ) intact. Ensure the `@ApiResponse`
references the same DTO type used in other endpoints so generated clients/docs
pick up the response shape.
In `@apps/api/src/app/agents/services/agent-config-resolver.service.ts`:
- Around line 39-44: The resolveReaction function currently performs an
unchecked cast that can crash later; change resolveReaction to validate the
incoming string against the known emoji set (e.g., DEFAULT_EMOJI_MAP or whatever
map/enum ChatSdkService.resolveEmoji uses) instead of blindly casting, and if
the value is missing from that map log a warning and return the provided
defaultEmoji (for undefined) or null (for explicit null) or a safe fallback for
unknown legacy values; reference resolveReaction and DEFAULT_EMOJI_MAP (and
ChatSdkService.resolveEmoji/getEmoji) to locate where to add the validation and
warning.
In `@apps/api/src/app/agents/services/chat-sdk.service.ts`:
- Around line 154-158: resolveEmoji currently returns whatever getEmoji(name)
yields (which can be undefined) and that undefined flows into
adapter.addReaction()/removeReaction() causing downstream failures; update the
resolveEmoji(name: string): Promise<EmojiValue> function to check the result of
getEmoji(name) and if falsy either throw a descriptive Error (e.g., `Unknown
emoji: ${name}`) or log a warning via the module logger before throwing,
ensuring callers receive a controlled error instead of passing undefined into
adapter.addReaction/removeReaction; reference getEmoji and
adapter.addReaction/removeReaction in your fix and keep the thrown message
explicit to aid debugging.
In
`@apps/api/src/app/agents/usecases/list-agent-emoji/list-agent-emoji.usecase.ts`:
- Around line 20-23: The mapping that builds this.cached in
list-agent-emoji.usecase.ts currently reads formats.gchat directly and can
produce undefined unicode values violating AgentEmojiEntry.unicode; update the
logic in the mapper inside the constructor or method that constructs this.cached
to compute unicode by trying formats.gchat ?? formats.slack ?? formats.teams ??
formats.whatsapp (or similar priority), and then filter out any entries where
the resolved unicode is still falsy so only entries with a valid string are
included; reference the existing map variable and the this.cached =
Object.entries(map).map(...) expression and ensure the result conforms to the
AgentEmojiEntry type.
In `@apps/api/src/app/agents/validators/is-well-known-emoji.validator.ts`:
- Around line 21-34: The defaultMessage in IsWellKnownEmojiConstraint currently
interpolates args.value directly which can produce unhelpful "[object Object]"
for non-string inputs; update the defaultMessage method to serialize the value
with JSON.stringify(args.value) (with a safe fallback like String(args.value) if
stringify fails or returns undefined) so error messages clearly show the
offending value; ensure you modify the defaultMessage implementation in the
IsWellKnownEmojiConstraint class (keep validate as-is).
In `@apps/dashboard/src/api/agents.ts`:
- Around line 266-268: The listAgentEmoji function is typed to return
Promise<AgentEmojiEntry[]> but the global ResponseInterceptor wraps responses in
an envelope, so change the call to use
get<AgentApiEnvelope<AgentEmojiEntry[]>>('/agents/emoji', { environment, signal
}) inside listAgentEmoji and return the unwrapped payload (response.data)
instead of the raw response; update listAgentEmoji to return
Promise<AgentEmojiEntry[]> by extracting .data so useAgentEmoji's emojiList
becomes an actual array.
In `@apps/dashboard/src/components/agents/setup-guide-primitives.tsx`:
- Around line 237-247: The confetti canvas uses fixed width/height from
window.innerWidth/innerHeight at render, so add a responsive size state and a
resize listener: in
apps/dashboard/src/components/agents/setup-guide-primitives.tsx create local
state (e.g., confettiSize with width/height), initialize from
window.innerWidth/innerHeight, update it in a debounced/window resize handler
inside useEffect and clean up the listener on unmount, then pass
confettiSize.width and confettiSize.height to the confetti props (replace the
current width={window.innerWidth} height={window.innerHeight}); keep
recycle={false} (or set recycle to true if you prefer automatic resizing) and
preserve numberOfPieces/style as-is.
- Around line 134-230: The component uses a manual useEffect + setInterval to
poll listAgentIntegrations and then calls queryClient.invalidateQueries for
getAgentIntegrationsQueryKey, which bypasses React Query; replace that polling
with a useQuery that uses getAgentIntegrationsQueryKey as the key and
listAgentIntegrations as the queryFn with refetchInterval: 1000 (and appropriate
enabled: !!currentEnvironment && !!watchedIntegrationId) so React Query handles
polling/cancellation/retries; move the confetti/onConnected logic into the
useQuery onSuccess handler (inspect the returned array to find the link by
watchedIntegrationId, setConnectedAt, fire setShowConfetti and schedule/clear
confettiTimeoutRef, and call onConnected once), and remove the manual interval,
cancelled flag, and invalidateQueries calls (let React Query be the single
source of truth).
In `@apps/dashboard/src/components/agents/teams-app-package.ts`:
- Around line 39-41: The Promise wrapping canvas.toBlob in teams-app-package.ts
resolves with blob! and ignores the case where canvas.toBlob yields null; modify
the Promise executor to accept (resolve, reject), and inside the canvas.toBlob
callback check if blob is null — if so call reject(new Error('Failed to create
blob from canvas')) (or a descriptive error) otherwise call resolve(blob);
update any callers if necessary to handle the rejection so the UI can surface a
toast instead of crashing when colorBlob.arrayBuffer() would be called on null.
- Around line 129-137: The synchronous URL.revokeObjectURL(url) can cancel the
download in some browsers; after creating the anchor (a) and calling a.click(),
defer revoking the object URL by calling URL.revokeObjectURL(url) on the next
tick (e.g. via setTimeout or requestAnimationFrame) or after a navigation event
so the download pipeline starts first — update the block that creates url, a,
sets a.download (safeName) and calls a.click() to revoke the URL asynchronously
instead of immediately.
In `@apps/dashboard/src/components/agents/teams-setup-guide.tsx`:
- Around line 46-83: The buildManifest function currently sets
webApplicationInfo.resource to api://${hostname}/${id}, includes a deprecated
top-level permissions array, and uses a white accentColor; update buildManifest
to (1) replace webApplicationInfo.resource with the bot's actual Azure AD
Application ID URI (use the registered resource format such as
api://botid-{appId} or api://{your-domain}/{appId} rather than
api://${hostname}/${id}) by deriving or accepting the correct resource string
when constructing webApplicationInfo.resource, (2) remove the top-level
permissions: ['identity','messageTeamMembers'] property and rely solely on
authorization.permissions.resourceSpecific (keep the existing authorization
object), and (3) change accentColor from '#FFFFFF' to a non-white branded hex
(e.g., '#0078D4' or your brand color) so it has sufficient contrast.
---
Nitpick comments:
In `@apps/api/src/app/agents/dtos/agent-behavior.dto.ts`:
- Line 16: Import WellKnownEmoji from 'chat' and use it as the TypeScript type
for the reaction fields instead of string | null (e.g., change the
onMessageReceived property's type to WellKnownEmoji | null); apply the same
change to any other emoji/reaction properties in the agent-behavior.dto.ts DTO
so the compile-time types match the runtime validators; ensure the new import is
added at the top of the file.
In `@apps/api/src/app/agents/services/chat-sdk.service.ts`:
- Line 126: Change the emojiName type from string to the WellKnownEmoji union at
the public API boundary so callers get compile-time safety: update the emojiName
property/parameter in ChatSdkService methods reactToMessage and removeReaction
(and any other occurrences at the same spots) to WellKnownEmoji, import or
re-export the WellKnownEmoji type where needed, and adjust any callers that pass
arbitrary strings to use a WellKnownEmoji value (or explicitly cast/validate
before calling) so resolveEmoji/getEmoji no longer receives unchecked strings.
In `@apps/dashboard/src/components/agents/teams-app-package.ts`:
- Line 22: The call to ctx.roundRect may fail in older browsers; add a runtime
fallback that checks for ctx.roundRect and if absent draws the rounded rectangle
manually using ctx.beginPath, ctx.moveTo/lineTo and ctx.arc (using the same size
and corner radius currently passed), then closePath and fill/stroke as
appropriate; update the block that currently calls ctx.roundRect(0, 0, size,
size, size * 0.18) to use this feature-detecting path so ctx.roundRect is used
when present and the manual arc-based drawing is used otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1237c639-5988-4982-995d-6daa63d5eac5
📒 Files selected for processing (24)
apps/api/src/app/agents/agents.controller.tsapps/api/src/app/agents/dtos/agent-behavior.dto.tsapps/api/src/app/agents/e2e/agent-webhook.e2e.tsapps/api/src/app/agents/services/agent-config-resolver.service.tsapps/api/src/app/agents/services/agent-inbound-handler.service.tsapps/api/src/app/agents/services/chat-sdk.service.tsapps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.command.tsapps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.usecase.tsapps/api/src/app/agents/usecases/index.tsapps/api/src/app/agents/usecases/list-agent-emoji/list-agent-emoji.usecase.tsapps/api/src/app/agents/utils/esm-import.tsapps/api/src/app/agents/validators/is-well-known-emoji.validator.tsapps/dashboard/src/api/agents.tsapps/dashboard/src/components/agents/agent-behavior-section.tsxapps/dashboard/src/components/agents/agent-integration-guides/resolve-agent-integration-guide.tsxapps/dashboard/src/components/agents/agent-integration-guides/teams-agent-integration-guide.tsxapps/dashboard/src/components/agents/agent-setup-guide.tsxapps/dashboard/src/components/agents/setup-guide-primitives.tsxapps/dashboard/src/components/agents/slack-setup-guide.tsxapps/dashboard/src/components/agents/teams-app-package.tsapps/dashboard/src/components/agents/teams-setup-guide.tsxapps/dashboard/src/components/agents/whatsapp-setup-guide.tsxpackages/shared/src/consts/providers/conversational-providers.tspackages/shared/src/consts/providers/credentials/provider-credentials.ts
| @Get('/emoji') | ||
| @ApiOperation({ | ||
| summary: 'List available emoji', | ||
| description: | ||
| 'Returns the set of well-known cross-platform emoji names supported for agent reactions. ' + | ||
| 'Each entry includes the normalized name and a unicode representation for display.', | ||
| }) | ||
| @RequirePermissions(PermissionsEnum.AGENT_READ) | ||
| listEmoji() { | ||
| return this.listAgentEmojiUsecase.execute(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Method name and missing @ApiResponse decorator.
Two guideline deviations:
- The method name
listEmojidoesn't match thelist{EntityName}pattern used elsewhere in this controller (listAgents,listAgentIntegrations). Rename tolistAgentEmojifor consistency. - The endpoint lacks a return type annotation and
@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
- `@Get`('/emoji')
- `@ApiOperation`({
- summary: 'List available emoji',
- description:
- 'Returns the set of well-known cross-platform emoji names supported for agent reactions. ' +
- 'Each entry includes the normalized name and a unicode representation for display.',
- })
- `@RequirePermissions`(PermissionsEnum.AGENT_READ)
- listEmoji() {
- return this.listAgentEmojiUsecase.execute();
- }
+ `@Get`('/emoji')
+ `@ApiOperation`({
+ summary: 'List available emoji',
+ description:
+ 'Returns the set of well-known cross-platform emoji names supported for agent reactions. ' +
+ 'Each entry includes the normalized name and a unicode representation for display.',
+ })
+ `@RequirePermissions`(PermissionsEnum.AGENT_READ)
+ listAgentEmoji(): Promise<AgentEmojiEntry[]> {
+ return this.listAgentEmojiUsecase.execute();
+ }As per coding guidelines: "Controller method names follow the pattern: getEntityName, listEntityName, ..." and "Every endpoint must have @ApiOperation, @ApiResponse, and @ApiTags decorators for OpenAPI documentation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/app/agents/agents.controller.ts` around lines 85 - 95, Rename
the controller method listEmoji to listAgentEmoji for consistency, update its
call to use listAgentEmojiUsecase.execute(), add an explicit return type (e.g.,
Promise<AgentEmojiDto[]> or the existing DTO type used elsewhere), and annotate
the method with an `@ApiResponse`(...) decorator describing the 200 response
schema and type; keep the existing `@ApiOperation` and
`@RequirePermissions`(PermissionsEnum.AGENT_READ) intact. Ensure the `@ApiResponse`
references the same DTO type used in other endpoints so generated clients/docs
pick up the response shape.
| export function ListeningStatus({ | ||
| agentIdentifier, | ||
| watchedIntegrationId, | ||
| onConnected, | ||
| connectedMessage, | ||
| listeningMessage, | ||
| }: { | ||
| agentIdentifier: string; | ||
| watchedIntegrationId: string | undefined; | ||
| onConnected?: () => void; | ||
| connectedMessage: string; | ||
| listeningMessage: string; | ||
| }) { | ||
| const { currentEnvironment } = useEnvironment(); | ||
| const queryClient = useQueryClient(); | ||
| const [connectedAt, setConnectedAt] = useState<string | null>(null); | ||
| const [showConfetti, setShowConfetti] = useState(false); | ||
| const confettiTimeoutRef = useRef<number | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| if (!currentEnvironment || !watchedIntegrationId) { | ||
| return; | ||
| } | ||
|
|
||
| const environment = currentEnvironment; | ||
| let cancelled = false; | ||
| let confettiFired = false; | ||
| let intervalId: ReturnType<typeof setInterval> | null = null; | ||
|
|
||
| async function tick() { | ||
| if (cancelled) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const res = await listAgentIntegrations({ | ||
| environment, | ||
| agentIdentifier, | ||
| limit: 100, | ||
| }); | ||
|
|
||
| if (cancelled) { | ||
| return; | ||
| } | ||
|
|
||
| const link = res.data.find((l) => l.integration._id === watchedIntegrationId); | ||
|
|
||
| if (!link?.connectedAt) { | ||
| return; | ||
| } | ||
|
|
||
| setConnectedAt(link.connectedAt); | ||
|
|
||
| if (!confettiFired) { | ||
| confettiFired = true; | ||
| setShowConfetti(true); | ||
|
|
||
| if (confettiTimeoutRef.current) { | ||
| clearTimeout(confettiTimeoutRef.current); | ||
| } | ||
|
|
||
| confettiTimeoutRef.current = window.setTimeout(() => { | ||
| confettiTimeoutRef.current = null; | ||
| setShowConfetti(false); | ||
| }, 10_000); | ||
| onConnected?.(); | ||
| } | ||
|
|
||
| queryClient.invalidateQueries({ | ||
| queryKey: getAgentIntegrationsQueryKey(environment._id, agentIdentifier), | ||
| }); | ||
|
|
||
| if (intervalId) { | ||
| clearInterval(intervalId); | ||
| intervalId = null; | ||
| } | ||
| } catch { | ||
| // ignore transient errors while polling | ||
| } | ||
| } | ||
|
|
||
| void tick(); | ||
| intervalId = setInterval(() => void tick(), 1000); | ||
|
|
||
| return () => { | ||
| cancelled = true; | ||
|
|
||
| if (intervalId) { | ||
| clearInterval(intervalId); | ||
| } | ||
|
|
||
| if (confettiTimeoutRef.current) { | ||
| clearTimeout(confettiTimeoutRef.current); | ||
| confettiTimeoutRef.current = null; | ||
| } | ||
| }; | ||
| }, [agentIdentifier, currentEnvironment, onConnected, queryClient, watchedIntegrationId]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace manual useEffect + setInterval polling with useQuery + refetchInterval.
The new shared ListeningStatus polls listAgentIntegrations manually and then invalidates the same getAgentIntegrationsQueryKey cache — so the component both bypasses React Query for its own reads and mutates the cache out-of-band. Using useQuery with refetchInterval gives you cancellation, focus/visibility-aware polling, retry/backoff, and a single source of truth for free, and lets other consumers of this query key benefit from the same data.
As per coding guidelines: "Use TanStack Query (useQuery, useMutation) for all server state — do not use useEffect + fetch directly".
♻️ Sketch of the refactor
- const { currentEnvironment } = useEnvironment();
- const queryClient = useQueryClient();
- const [connectedAt, setConnectedAt] = useState<string | null>(null);
- const [showConfetti, setShowConfetti] = useState(false);
- const confettiTimeoutRef = useRef<number | null>(null);
-
- useEffect(() => {
- if (!currentEnvironment || !watchedIntegrationId) {
- return;
- }
- // ...manual setInterval/tick/cancelled machinery...
- }, [agentIdentifier, currentEnvironment, onConnected, queryClient, watchedIntegrationId]);
+ const { currentEnvironment } = useEnvironment();
+ const [showConfetti, setShowConfetti] = useState(false);
+ const confettiTimeoutRef = useRef<number | null>(null);
+ const onConnectedRef = useRef(onConnected);
+ onConnectedRef.current = onConnected;
+
+ const { data } = useQuery({
+ queryKey: currentEnvironment
+ ? getAgentIntegrationsQueryKey(currentEnvironment._id, agentIdentifier)
+ : ['agent-integrations', 'disabled'],
+ queryFn: () =>
+ listAgentIntegrations({
+ environment: currentEnvironment!,
+ agentIdentifier,
+ limit: 100,
+ }),
+ enabled: Boolean(currentEnvironment && watchedIntegrationId),
+ refetchInterval: (query) => {
+ const link = query.state.data?.data.find((l) => l.integration._id === watchedIntegrationId);
+
+ return link?.connectedAt ? false : 1000;
+ },
+ });
+
+ const connectedAt = data?.data.find((l) => l.integration._id === watchedIntegrationId)?.connectedAt ?? null;
+
+ useEffect(() => {
+ if (!connectedAt) return;
+ setShowConfetti(true);
+ onConnectedRef.current?.();
+ confettiTimeoutRef.current = window.setTimeout(() => setShowConfetti(false), 10_000);
+
+ return () => {
+ if (confettiTimeoutRef.current) clearTimeout(confettiTimeoutRef.current);
+ };
+ }, [connectedAt]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/agents/setup-guide-primitives.tsx` around lines
134 - 230, The component uses a manual useEffect + setInterval to poll
listAgentIntegrations and then calls queryClient.invalidateQueries for
getAgentIntegrationsQueryKey, which bypasses React Query; replace that polling
with a useQuery that uses getAgentIntegrationsQueryKey as the key and
listAgentIntegrations as the queryFn with refetchInterval: 1000 (and appropriate
enabled: !!currentEnvironment && !!watchedIntegrationId) so React Query handles
polling/cancellation/retries; move the confetti/onConnected logic into the
useQuery onSuccess handler (inspect the returned array to find the link by
watchedIntegrationId, setConnectedAt, fire setShowConfetti and schedule/clear
confettiTimeoutRef, and call onConnected once), and remove the manual interval,
cancelled flag, and invalidateQueries calls (let React Query be the single
source of truth).
| width={window.innerWidth} | ||
| height={window.innerHeight} | ||
| recycle={false} | ||
| numberOfPieces={1000} | ||
| style={{ | ||
| position: 'fixed', | ||
| inset: 0, | ||
| pointerEvents: 'none', | ||
| zIndex: 10000, | ||
| }} | ||
| />, |
There was a problem hiding this comment.
Confetti canvas size doesn't react to window resize.
width={window.innerWidth} / height={window.innerHeight} are captured once at render. If the viewport changes during the ~10s confetti window (rotate device, DevTools toggle), pieces will fall outside the canvas. Minor, but trivial to fix by tracking size in state or passing recycle={false} + a resize listener.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/agents/setup-guide-primitives.tsx` around lines
237 - 247, The confetti canvas uses fixed width/height from
window.innerWidth/innerHeight at render, so add a responsive size state and a
resize listener: in
apps/dashboard/src/components/agents/setup-guide-primitives.tsx create local
state (e.g., confettiSize with width/height), initialize from
window.innerWidth/innerHeight, update it in a debounced/window resize handler
inside useEffect and clean up the listener on unmount, then pass
confettiSize.width and confettiSize.height to the confetti props (replace the
current width={window.innerWidth} height={window.innerHeight}); keep
recycle={false} (or set recycle to true if you prefer automatic resizing) and
preserve numberOfPieces/style as-is.
| return new Promise((resolve) => { | ||
| canvas.toBlob((blob) => resolve(blob!), 'image/png'); | ||
| }); |
There was a problem hiding this comment.
canvas.toBlob can return null — promise will hang / downstream crash.
toBlob passes null when encoding fails (e.g. tainted canvas, OOM). With resolve(blob!), the consumer later calls await colorBlob.arrayBuffer() on null, throwing a cryptic TypeError. Reject explicitly so the caller can surface a toast.
🛡️ Proposed fix
- return new Promise((resolve) => {
- canvas.toBlob((blob) => resolve(blob!), 'image/png');
- });
+ return new Promise((resolve, reject) => {
+ canvas.toBlob((blob) => {
+ if (blob) {
+ resolve(blob);
+ } else {
+ reject(new Error('Failed to encode canvas to PNG'));
+ }
+ }, 'image/png');
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/agents/teams-app-package.ts` around lines 39 -
41, The Promise wrapping canvas.toBlob in teams-app-package.ts resolves with
blob! and ignores the case where canvas.toBlob yields null; modify the Promise
executor to accept (resolve, reject), and inside the canvas.toBlob callback
check if blob is null — if so call reject(new Error('Failed to create blob from
canvas')) (or a descriptive error) otherwise call resolve(blob); update any
callers if necessary to handle the rejection so the UI can surface a toast
instead of crashing when colorBlob.arrayBuffer() would be called on null.
| const url = URL.createObjectURL(zip); | ||
| const a = document.createElement('a'); | ||
| a.href = url; | ||
| const safeName = (agentName || '').toLowerCase().replace(/\s+/g, '-').replace(/[^a-z0-9-]/g, '') || 'novu-agent'; | ||
| a.download = `${safeName}-teams-app.zip`; | ||
| document.body.appendChild(a); | ||
| a.click(); | ||
| a.remove(); | ||
| URL.revokeObjectURL(url); |
There was a problem hiding this comment.
Revoking the object URL synchronously after a.click() can cancel the download.
On some browsers (notably Firefox and some Safari versions) URL.revokeObjectURL runs before the download pipeline has fully started when invoked on the same tick as click(), producing an empty/failed save. Defer the revoke to the next tick (or on the a’s navigate event).
🛡️ Proposed fix
document.body.appendChild(a);
a.click();
a.remove();
- URL.revokeObjectURL(url);
+ // Defer revoke so the browser has time to start the download.
+ setTimeout(() => URL.revokeObjectURL(url), 0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const url = URL.createObjectURL(zip); | |
| const a = document.createElement('a'); | |
| a.href = url; | |
| const safeName = (agentName || '').toLowerCase().replace(/\s+/g, '-').replace(/[^a-z0-9-]/g, '') || 'novu-agent'; | |
| a.download = `${safeName}-teams-app.zip`; | |
| document.body.appendChild(a); | |
| a.click(); | |
| a.remove(); | |
| URL.revokeObjectURL(url); | |
| const url = URL.createObjectURL(zip); | |
| const a = document.createElement('a'); | |
| a.href = url; | |
| const safeName = (agentName || '').toLowerCase().replace(/\s+/g, '-').replace(/[^a-z0-9-]/g, '') || 'novu-agent'; | |
| a.download = `${safeName}-teams-app.zip`; | |
| document.body.appendChild(a); | |
| a.click(); | |
| a.remove(); | |
| // Defer revoke so the browser has time to start the download. | |
| setTimeout(() => URL.revokeObjectURL(url), 0); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/agents/teams-app-package.ts` around lines 129 -
137, The synchronous URL.revokeObjectURL(url) can cancel the download in some
browsers; after creating the anchor (a) and calling a.click(), defer revoking
the object URL by calling URL.revokeObjectURL(url) on the next tick (e.g. via
setTimeout or requestAnimationFrame) or after a navigation event so the download
pipeline starts first — update the block that creates url, a, sets a.download
(safeName) and calls a.click() to revoke the URL asynchronously instead of
immediately.
| function buildManifest(appId: string, agentName: string): Record<string, unknown> { | ||
| const id = appId || 'YOUR_APP_ID'; | ||
| const name = agentName || 'Novu Agent'; | ||
| const hostname = getApiHostname(); | ||
|
|
||
| return { | ||
| $schema: 'https://developer.microsoft.com/json-schemas/teams/v1.16/MicrosoftTeams.schema.json', | ||
| manifestVersion: '1.16', | ||
| version: '1.0.0', | ||
| id, | ||
| developer: { | ||
| name: 'Your Company', | ||
| websiteUrl: 'https://your-domain.com', | ||
| privacyUrl: 'https://your-domain.com/privacy', | ||
| termsOfUseUrl: 'https://your-domain.com/terms', | ||
| }, | ||
| name: { short: name, full: `${name} — powered by Novu` }, | ||
| description: { short: `${name} bot`, full: 'A conversational agent powered by Novu.' }, | ||
| icons: { outline: 'outline.png', color: 'color.png' }, | ||
| accentColor: '#FFFFFF', | ||
| bots: [ | ||
| { | ||
| botId: id, | ||
| scopes: ['personal', 'team', 'groupchat'], | ||
| supportsFiles: false, | ||
| isNotificationOnly: false, | ||
| }, | ||
| ], | ||
| permissions: ['identity', 'messageTeamMembers'], | ||
| validDomains: [hostname], | ||
| webApplicationInfo: { id, resource: `api://${hostname}/${id}` }, | ||
| authorization: { | ||
| permissions: { | ||
| resourceSpecific: [{ name: 'ChannelMessage.Read.Group', type: 'Application' }], | ||
| }, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Microsoft Teams app manifest v1.16 webApplicationInfo.resource format requirements for Azure Bot
💡 Result:
In Microsoft Teams app manifest version 1.16, the webApplicationInfo object is used to enable Single Sign-On (SSO) for bots and other capabilities. It has two required properties: - id: The Application (client) ID of the Microsoft Entra ID (formerly Azure AD) app registration associated with the Azure Bot, in GUID format. - resource: The Application ID URI of the Entra ID app registration, which must exactly match the URI configured in the "Expose an API" section of the app registration. For Azure Bots: - Standalone bot (no tabs): Use "api://botid-{bot-app-id}" where {bot-app-id} is the Microsoft Entra app ID (often the same as the botId). - Bot with tabs: Use "api://{domain}/{bot-app-id}", where {domain} is a fully qualified domain name (FQDN) like "bot.contoso.com". This is required for tab SSO, and using api://{bot-app-id} alone will cause permission revocation after installation. The resource field is a string with no additional schema format restrictions beyond matching the Entra ID configuration. It enables seamless authentication via getAuthToken and is required for features like Resource-Specific Consent (RSC) permissions. Example for standalone Azure Bot: "webApplicationInfo": { "id": "12345678-1234-1234-1234-123456789abc", "resource": "api://botid-12345678-1234-1234-1234-123456789abc" } Example for bot with tabs: "webApplicationInfo": { "id": "12345678-1234-1234-1234-123456789abc", "resource": "api://bot.contoso.com/12345678-1234-1234-1234-123456789abc" } Always ensure the Entra ID app registration's Application ID URI matches the resource value exactly. Use manifest version 1.5 or later (1.16 is supported, released February 2023).
Citations:
- 1: https://learn.microsoft.com/en-us/microsoftteams/platform/resources/schema/manifest-schema
- 2: https://aka.ms/teams-manifest-schema
- 3: https://learn.microsoft.com/en-us/answers/questions/5810892/microsoft-teams-bot-stops-receiving-events-after-u
- 4: https://learn.microsoft.com/en-us/microsoftteams/platform/bots/how-to/authentication/bot-sso-register-aad
- 5: https://learn.microsoft.com/en-us/microsoftteams/platform/bots/how-to/conversations/channel-messages-for-bots-and-agents
- 6: https://learn.microsoft.com/en-us/microsoftteams/platform/bots/how-to/authentication/bot-sso-manifest
- 7: https://github.com/OfficeDev/microsoft-teams-app-schema
- 8: https://github.com/microsoft/json-schemas/tree/main/teams
- 9: https://raw.githubusercontent.com/OfficeDev/microsoft-teams-app-schema/preview/DevPreview/MicrosoftTeams.schema.json
🌐 Web query:
Microsoft Teams manifest permissions field deprecated authorization.permissions resourceSpecific
💡 Result:
The "permissions" field (top-level array like "permissions": ["identity", "messageTeamMembers"]) in the Microsoft Teams app manifest is deprecated. It has been deprecated for approximately 3 years as of early 2025. Developers should use the "authorization.permissions.resourceSpecific" structure instead for declaring Resource-Specific Consent (RSC) permissions. The top-level "permissions" array was used in older manifest versions for basic permissions. With the introduction of RSC (starting around manifest v1.6 for application permissions, v1.12 for delegated), the new "authorization" object under root contains "permissions" with "resourceSpecific" array of objects like {"name": "ChannelMessage.Read.Group", "type": "Application"}. Official schema docs up to v1.26 (April 2026) actively document and exemplify the authorization.permissions.resourceSpecific path, while samples using the old "permissions" are being flagged for update. No enforcement of removal yet, but recommended to migrate for RSC support and future compatibility.
Citations:
- 1: Feedback - Deprecated permission section of the manifest used in the sample MicrosoftDocs/msteams-docs#12175
- 2: https://learn.microsoft.com/en-us/microsoftteams/platform/resources/schema/manifest-schema
- 3: https://learn.microsoft.com/en-us/microsoftteams/platform/graph-api/rsc/resource-specific-consent
- 4: https://learn.microsoft.com/en-us/microsoft-365/extensibility/schema/root-authorization-permissions-resource-specific?view=m365-app-1.26
- 5: https://learn.microsoft.com/en-us/microsoftteams/platform/graph-api/rsc/grant-resource-specific-consent
- 6: https://learn.microsoft.com/en-us/microsoft-365/extensibility/schema/?view=m365-app-1.26
- 7: https://github.com/OfficeDev/microsoft-teams-app-schema
Fix Teams manifest: incorrect resource URI, deprecated permissions, and low-contrast accent color.
The manifest generator has three issues affecting Teams app validation and function:
-
Line 76 —
webApplicationInfo.resource: "api://${hostname}/${id}"uses the Novu API hostname, but must match the Azure AD app registration's actual Application ID URI (e.g.,"api://botid-{appId}"for standalone bots, or"api://{your-domain}/{appId}"for bots with tabs). Teams will fail to request tokens for the wrong resource URI, breaking SSO and getAuthToken calls. Update to use the bot's registered Azure AD resource URI. -
Line 74 — Remove the top-level
permissions: ['identity', 'messageTeamMembers']array. This field is deprecated and conflicts with the newerauthorization.permissions.resourceSpecificstructure already present (lines 77–81). Having both causes Partner Center and App Studio validators to reject the submission. Keep only theauthorizationobject. -
Line 65 —
accentColor: '#FFFFFF'renders invisible against Teams' light theme. Use a branded, non-white hex value (e.g.,'#0078D4'for Microsoft blue, or your brand color).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/agents/teams-setup-guide.tsx` around lines 46 -
83, The buildManifest function currently sets webApplicationInfo.resource to
api://${hostname}/${id}, includes a deprecated top-level permissions array, and
uses a white accentColor; update buildManifest to (1) replace
webApplicationInfo.resource with the bot's actual Azure AD Application ID URI
(use the registered resource format such as api://botid-{appId} or
api://{your-domain}/{appId} rather than api://${hostname}/${id}) by deriving or
accepting the correct resource string when constructing
webApplicationInfo.resource, (2) remove the top-level permissions:
['identity','messageTeamMembers'] property and rely solely on
authorization.permissions.resourceSpecific (keep the existing authorization
object), and (3) change accentColor from '#FFFFFF' to a non-white branded hex
(e.g., '#0078D4' or your brand color) so it has sufficient contrast.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/api/src/app/agents/agents.controller.ts (1)
85-95:⚠️ Potential issue | 🟡 Minor
listAgentEmojistill missing@ApiResponse.The rename to
listAgentEmojiand explicitPromise<AgentEmojiEntry[]>return type are in, but the endpoint still lacks an@ApiResponse(...)decorator. Every other handler in this controller declares one, and the coding guideline requires@ApiOperation,@ApiResponse, and@ApiTagson every endpoint even though the controller is@ApiExcludeController. Attach an@ApiResponsethat describes theAgentEmojiEntry[]shape (you'll likely need a small DTO class, since@ApiResponsetypically expects a class reference rather than a bareinterface, to keep generated clients/dashboards in sync).As per coding guidelines: "Every endpoint must have
@ApiOperation,@ApiResponse, and@ApiTagsdecorators for OpenAPI documentation".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/agents.controller.ts` around lines 85 - 95, The endpoint listAgentEmoji is missing an `@ApiResponse` decorator describing the returned AgentEmojiEntry[]; add an `@ApiResponse`({ status: 200, type: AgentEmojiDto, isArray: true, description: 'List of agent emoji entries' }) above the listAgentEmoji() method and create a small DTO class AgentEmojiDto (mirroring AgentEmojiEntry fields) to use as the class reference for OpenAPI generation; ensure the new DTO is imported where listAgentEmoji is defined and keep the existing return type Promise<AgentEmojiEntry[]> and call to listAgentEmojiUsecase.execute().
🧹 Nitpick comments (1)
apps/api/src/app/agents/services/agent-config-resolver.service.ts (1)
16-27: DRY: duplicate emoji-name cache/loader with the validator.
loadEmojiNames()and its module-levelcachedEmojiNameshere are a near-verbatim copy of the same logic inapps/api/src/app/agents/validators/is-well-known-emoji.validator.ts. Two independent caches also means the emoji set is materialized twice per process. Consider extracting a single shared helper (e.g.utils/well-known-emoji.tsexportingloadEmojiNames()) and reusing it from both sites.Additionally, the Biome hint on line 1 is valid for the module-level
new Logger('AgentConfigResolver')on line 16 — the injectedPinoLoggeris preferred. Easiest path is to moveresolveReactioninto theAgentConfigResolverclass (or a helper that accepts a logger) sothis.logger.warn(...)can be used instead of a standaloneLoggerinstance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/services/agent-config-resolver.service.ts` around lines 16 - 27, The module duplicates the emoji-name cache/loader (cachedEmojiNames + loadEmojiNames) already present in is-well-known-emoji.validator.ts and also uses a standalone new Logger('AgentConfigResolver') instead of the injected PinoLogger; refactor by extracting the emoji loader into a single shared helper (e.g. utils/well-known-emoji.ts) that exports loadEmojiNames() which imports DEFAULT_EMOJI_MAP and manages a single cachedEmojiNames Set, then replace the local cachedEmojiNames/loadEmojiNames usage in AgentConfigResolver and is-well-known-emoji.validator.ts with that shared helper; also move resolveReaction into the AgentConfigResolver class (or into a helper that accepts a logger) so you can use this.logger.warn (the injected PinoLogger) instead of the module-level new Logger('AgentConfigResolver').
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/api/src/app/agents/agents.controller.ts`:
- Around line 85-95: The endpoint listAgentEmoji is missing an `@ApiResponse`
decorator describing the returned AgentEmojiEntry[]; add an `@ApiResponse`({
status: 200, type: AgentEmojiDto, isArray: true, description: 'List of agent
emoji entries' }) above the listAgentEmoji() method and create a small DTO class
AgentEmojiDto (mirroring AgentEmojiEntry fields) to use as the class reference
for OpenAPI generation; ensure the new DTO is imported where listAgentEmoji is
defined and keep the existing return type Promise<AgentEmojiEntry[]> and call to
listAgentEmojiUsecase.execute().
---
Nitpick comments:
In `@apps/api/src/app/agents/services/agent-config-resolver.service.ts`:
- Around line 16-27: The module duplicates the emoji-name cache/loader
(cachedEmojiNames + loadEmojiNames) already present in
is-well-known-emoji.validator.ts and also uses a standalone new
Logger('AgentConfigResolver') instead of the injected PinoLogger; refactor by
extracting the emoji loader into a single shared helper (e.g.
utils/well-known-emoji.ts) that exports loadEmojiNames() which imports
DEFAULT_EMOJI_MAP and manages a single cachedEmojiNames Set, then replace the
local cachedEmojiNames/loadEmojiNames usage in AgentConfigResolver and
is-well-known-emoji.validator.ts with that shared helper; also move
resolveReaction into the AgentConfigResolver class (or into a helper that
accepts a logger) so you can use this.logger.warn (the injected PinoLogger)
instead of the module-level new Logger('AgentConfigResolver').
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e8a4a949-a377-4078-9365-7730c60f6ef9
📒 Files selected for processing (6)
apps/api/src/app/agents/agents.controller.tsapps/api/src/app/agents/services/agent-config-resolver.service.tsapps/api/src/app/agents/services/chat-sdk.service.tsapps/api/src/app/agents/usecases/list-agent-emoji/list-agent-emoji.usecase.tsapps/api/src/app/agents/validators/is-well-known-emoji.validator.tsapps/dashboard/src/api/agents.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/dashboard/src/api/agents.ts
- apps/api/src/app/agents/usecases/list-agent-emoji/list-agent-emoji.usecase.ts
- apps/api/src/app/agents/services/chat-sdk.service.ts
- apps/api/src/app/agents/validators/is-well-known-emoji.validator.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/api/src/app/agents/services/chat-sdk.service.ts (1)
327-340: Nit: document theevent.thread as Thread | undefinedcast.The commit message attributes this to "generic variance mismatches" in the
chatSDK types. A one-line comment next to the cast will save the next maintainer agit blamehop and makes it easier to remove once the upstream generics are tightened.- thread: event.thread as Thread | undefined, + // ReactionEvent.thread is generic over adapter type; cast narrows to the + // concrete Thread used downstream until upstream variance is tightened. + thread: event.thread as Thread | undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/services/chat-sdk.service.ts` around lines 327 - 340, Add an inline one-line comment explaining the cast of event.thread to Thread | undefined to document the generic variance workaround: next to the cast in the chat.onReaction handler (the line with event.thread as Thread | undefined used when calling this.inboundHandler.handleReaction), add a brief note referencing "generic variance mismatches in chat SDK types" (or similar) and that it can be removed once upstream generics are fixed; keep the comment concise and colocated with the cast so future maintainers (and git blame) see the rationale immediately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/agents/services/chat-sdk.service.ts`:
- Around line 154-162: Replace the plain Error thrown in resolveEmoji with
Nest's BadRequestException (imported from `@nestjs/common`) so invalid emoji names
produce HTTP 400; also avoid repeated ESM resolution by caching the imported
getEmoji function (either memoize esmImport result or lift getEmoji to a
module-level variable) so resolveEmoji uses the cached getEmoji on subsequent
calls; update resolveEmoji, and ensure callers reactToMessage and removeReaction
benefit from the cached function while keeping the `@IsWellKnownEmoji` validator
as defense-in-depth.
---
Nitpick comments:
In `@apps/api/src/app/agents/services/chat-sdk.service.ts`:
- Around line 327-340: Add an inline one-line comment explaining the cast of
event.thread to Thread | undefined to document the generic variance workaround:
next to the cast in the chat.onReaction handler (the line with event.thread as
Thread | undefined used when calling this.inboundHandler.handleReaction), add a
brief note referencing "generic variance mismatches in chat SDK types" (or
similar) and that it can be removed once upstream generics are fixed; keep the
comment concise and colocated with the cast so future maintainers (and git
blame) see the rationale immediately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aea5bcb1-6aa2-4520-9fb7-97eb14695eb2
📒 Files selected for processing (2)
apps/api/src/app/agents/services/chat-sdk.service.tsapps/api/src/app/agents/usecases/list-agent-emoji/list-agent-emoji.usecase.ts
✅ Files skipped from review due to trivial changes (1)
- apps/api/src/app/agents/usecases/list-agent-emoji/list-agent-emoji.usecase.ts
| 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; | ||
| } |
There was a problem hiding this comment.
🧩 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 BadRequestException for invalid emoji names and consider caching the emoji module to avoid repeated ESM resolution on hot paths.
Two confirmed issues in resolveEmoji:
-
Throwing a plain
Errorsurfaces as a 500 instead of a 400. Since the rejection is due to an invalid client-supplied value,BadRequestExceptionis semantically correct and consistent with the pattern used throughout this file (lines 63, 236, 250, 265, 282). The@IsWellKnownEmojivalidator on the DTO provides defense-in-depth, but programmatic callers and SDK-version mismatches can still reach this path. -
resolveEmojiis awaited on everyreactToMessageandremoveReactioncall. TheesmImportutility has no memoization—it's a thin wrapper aroundimport()—so each call incurs ESM resolution overhead on these hot paths. While the validator already caches emoji names,resolveEmojishould cache thegetEmojifunction or be lifted to a module-level reference.
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
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/app/agents/services/chat-sdk.service.ts` around lines 154 - 162,
Replace the plain Error thrown in resolveEmoji with Nest's BadRequestException
(imported from `@nestjs/common`) so invalid emoji names produce HTTP 400; also
avoid repeated ESM resolution by caching the imported getEmoji function (either
memoize esmImport result or lift getEmoji to a module-level variable) so
resolveEmoji uses the cached getEmoji on subsequent calls; update resolveEmoji,
and ensure callers reactToMessage and removeReaction benefit from the cached
function while keeping the `@IsWellKnownEmoji` validator as defense-in-depth.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/src/app/agents/services/agent-config-resolver.service.ts (1)
16-25: Cache the in-flight promise to avoid duplicate SDK imports on concurrent cold starts.
cachedEmojiNamesis populated only afterawait esmImport('chat')resolves. IfresolveReactionis invoked concurrently (likely, sinceresolve()awaits both reactions in sequence but multiple agents may resolve in parallel) before the first call completes, each caller re-enters theesmImportbranch and re-does the dynamic import +Object.keys. Result is idempotent, but a one-line change removes the wasted work.♻️ Proposed fix — cache the Promise
-let cachedEmojiNames: Set<string> | null = null; - -async function loadEmojiNames(): Promise<Set<string>> { - if (cachedEmojiNames) return cachedEmojiNames; - - const { DEFAULT_EMOJI_MAP } = await esmImport('chat'); - cachedEmojiNames = new Set<string>(Object.keys(DEFAULT_EMOJI_MAP)); - - return cachedEmojiNames; -} +let cachedEmojiNamesPromise: Promise<Set<string>> | null = null; + +function loadEmojiNames(): Promise<Set<string>> { + if (!cachedEmojiNamesPromise) { + cachedEmojiNamesPromise = esmImport('chat').then( + ({ DEFAULT_EMOJI_MAP }) => new Set<string>(Object.keys(DEFAULT_EMOJI_MAP)) + ); + } + + return cachedEmojiNamesPromise; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/services/agent-config-resolver.service.ts` around lines 16 - 25, The loadEmojiNames function currently sets cachedEmojiNames only after awaiting esmImport('chat'), causing concurrent callers to trigger duplicate imports; fix by caching the in-flight Promise: replace or add a variable (e.g., cachedEmojiNamesPromise: Promise<Set<string>> | null) and assign it immediately when first entering loadEmojiNames (const p = esmImport('chat').then(m => new Set(Object.keys(m.DEFAULT_EMOJI_MAP))); cachedEmojiNamesPromise = p; cachedEmojiNames = await p; return cachedEmojiNames; ensure you clear cachedEmojiNamesPromise and cachedEmojiNames on rejection so failed imports don't poison the cache; keep references to cachedEmojiNames, loadEmojiNames, esmImport and DEFAULT_EMOJI_MAP when making changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/api/src/app/agents/services/agent-config-resolver.service.ts`:
- Around line 16-25: The loadEmojiNames function currently sets cachedEmojiNames
only after awaiting esmImport('chat'), causing concurrent callers to trigger
duplicate imports; fix by caching the in-flight Promise: replace or add a
variable (e.g., cachedEmojiNamesPromise: Promise<Set<string>> | null) and assign
it immediately when first entering loadEmojiNames (const p =
esmImport('chat').then(m => new Set(Object.keys(m.DEFAULT_EMOJI_MAP)));
cachedEmojiNamesPromise = p; cachedEmojiNames = await p; return
cachedEmojiNames; ensure you clear cachedEmojiNamesPromise and cachedEmojiNames
on rejection so failed imports don't poison the cache; keep references to
cachedEmojiNames, loadEmojiNames, esmImport and DEFAULT_EMOJI_MAP when making
changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eb992874-ff76-48eb-aaaf-87c5c3d1c6e9
📒 Files selected for processing (1)
apps/api/src/app/agents/services/agent-config-resolver.service.ts
…nt reactions Replace hardcoded emoji strings with the Chat SDK's type-safe emoji system. The SDK's WellKnownEmoji type, EmojiValue singletons, and DEFAULT_EMOJI_MAP are now the single source of truth for cross-platform emoji support. Key changes: - Use getEmoji() to resolve emoji names to EmojiValue before passing to platform adapters, ensuring correct per-platform conversion - Type ResolvedAgentConfig reaction fields as WellKnownEmoji | null - Type onReaction handler as ReactionEvent (removes `any` cast) - Add @IsWellKnownEmoji() DTO validator (lazily loads SDK emoji map) - Add GET /agents/emoji endpoint so the dashboard can discover available emoji - Dashboard fetches emoji unicode from the API instead of hardcoding - Extract shared esmImport helper to utils/esm-import.ts - Rename executeResolveSignal → resolveConversation (resolve is not a signal) Made-with: Cursor
- Rename listEmoji → listAgentEmoji with explicit return type for
controller naming consistency
- Validate legacy DB emoji values against DEFAULT_EMOJI_MAP in
resolveReaction, falling back to default with a warning log
- Guard resolveEmoji against unknown names (getEmoji can return undefined)
- Add fallback chain (gchat → slack → teams → whatsapp) in ListAgentEmoji
and filter out entries with no unicode representation
- Use JSON.stringify in validator error message for non-string values
- Fix critical bug: unwrap ResponseInterceptor envelope in dashboard
listAgentEmoji (was receiving { data: [...] } instead of [...])
Made-with: Cursor
- Remove non-existent 'teams' and 'whatsapp' from EmojiFormats fallback chain — the chat SDK type only has gchat and slack fields - Cast ReactionEvent.thread to Thread to resolve generic variance mismatch (Thread<unknown> vs Thread<Record<string, unknown>>) Made-with: Cursor
…eaction Biome lint forbids importing Logger from @nestjs/common — the project convention requires PinoLogger from @novu/application-generic. Pass the class-level PinoLogger instance into the standalone function. Made-with: Cursor
ChmaraX
force-pushed
the
feat/adopt-chat-sdk-emoji-system
branch
from
6100435 to
193d0db
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/app/agents/services/agent-inbound-handler.service.ts (1)
24-31:⚠️ Potential issue | 🔴 CriticalFix type mismatch:
EmojiValuedoes not expose anamefield.Line 236 reads
event.emoji.name, butEmojiValue(from the Chat SDK atchatv4.25.0) is the resolved emoji object itself, not an object with anameproperty. The inline type{ name: string }was replaced withEmojiValue, but the code still attempts to access.nameon it. At runtime,event.emoji.namewill beundefined, breaking the reaction handler and leavingBridgeReaction.emojiempty.Either restore the inline
{ name: string }shape forInboundReactionEvent.emojiand extract the name separately from the chat SDK'sReactionEvent, or determine the correct property/property path fromEmojiValueto pass toBridgeReaction.emoji.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/services/agent-inbound-handler.service.ts` around lines 24 - 31, InboundReactionEvent currently types emoji as EmojiValue but code still reads event.emoji.name (used when building BridgeReaction.emoji) which is incorrect; either restore the inline shape (emoji: { name: string }) or keep EmojiValue and map its actual field (inspect the chat SDK EmojiValue shape in the dependency) to supply the expected string to BridgeReaction.emoji in the reaction handler in agent-inbound-handler.service.ts (where event.emoji.name is referenced). Update the InboundReactionEvent interface or the extraction logic so BridgeReaction.emoji is assigned the correct emoji string from the real EmojiValue property.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/src/components/agents/agent-behavior-section.tsx`:
- Around line 64-68: AgentBehaviorSection currently reads emoji via
useAgentEmoji and falls back to '' which renders an empty EmojiPickerButton
while listAgentEmoji is loading/fails; update AgentBehaviorSection to (1) read
isLoading/isError from useAgentEmoji and show a skeleton/placeholder component
for EmojiPickerButton while isLoading, and (2) provide a static unicode fallback
for DEFAULT_REACTION_ON_MESSAGE and DEFAULT_REACTION_ON_RESOLVED (lookup by
name) when unicodeMap.get(...) returns undefined or isError is true so the UI
shows meaningful emojis offline; locate logic around useAgentEmoji, unicodeMap,
DEFAULT_REACTION_ON_MESSAGE, DEFAULT_REACTION_ON_RESOLVED and the
EmojiPickerButton usage to implement these checks and fallbacks.
---
Outside diff comments:
In `@apps/api/src/app/agents/services/agent-inbound-handler.service.ts`:
- Around line 24-31: InboundReactionEvent currently types emoji as EmojiValue
but code still reads event.emoji.name (used when building BridgeReaction.emoji)
which is incorrect; either restore the inline shape (emoji: { name: string }) or
keep EmojiValue and map its actual field (inspect the chat SDK EmojiValue shape
in the dependency) to supply the expected string to BridgeReaction.emoji in the
reaction handler in agent-inbound-handler.service.ts (where event.emoji.name is
referenced). Update the InboundReactionEvent interface or the extraction logic
so BridgeReaction.emoji is assigned the correct emoji string from the real
EmojiValue property.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ee6defd7-3dfa-4785-8430-d0f8519856dc
📒 Files selected for processing (14)
apps/api/src/app/agents/agents.controller.tsapps/api/src/app/agents/dtos/agent-behavior.dto.tsapps/api/src/app/agents/e2e/agent-webhook.e2e.tsapps/api/src/app/agents/services/agent-config-resolver.service.tsapps/api/src/app/agents/services/agent-inbound-handler.service.tsapps/api/src/app/agents/services/chat-sdk.service.tsapps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.command.tsapps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.usecase.tsapps/api/src/app/agents/usecases/index.tsapps/api/src/app/agents/usecases/list-agent-emoji/list-agent-emoji.usecase.tsapps/api/src/app/agents/utils/esm-import.tsapps/api/src/app/agents/validators/is-well-known-emoji.validator.tsapps/dashboard/src/api/agents.tsapps/dashboard/src/components/agents/agent-behavior-section.tsx
✅ Files skipped from review due to trivial changes (6)
- apps/api/src/app/agents/usecases/index.ts
- apps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.command.ts
- apps/api/src/app/agents/e2e/agent-webhook.e2e.ts
- apps/api/src/app/agents/utils/esm-import.ts
- apps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.usecase.ts
- apps/api/src/app/agents/usecases/list-agent-emoji/list-agent-emoji.usecase.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/src/app/agents/services/agent-config-resolver.service.ts
- apps/api/src/app/agents/validators/is-well-known-emoji.validator.ts
| export function AgentBehaviorSection() { | ||
| const { unicodeMap } = useAgentEmoji(); | ||
| const messageEmoji = unicodeMap.get(DEFAULT_REACTION_ON_MESSAGE) ?? ''; | ||
| const resolvedEmoji = unicodeMap.get(DEFAULT_REACTION_ON_RESOLVED) ?? ''; | ||
|
|
There was a problem hiding this comment.
Empty emoji during initial load and on fetch failure.
messageEmoji/resolvedEmoji fall back to '' until listAgentEmoji resolves (or if it fails), causing EmojiPickerButton to render an empty pill before React Query populates unicodeMap. Consider either (a) rendering a skeleton/placeholder while isLoading, or (b) falling back to a static unicode for the two well-known names so the UI is meaningful offline or during error.
As per coding guidelines for apps/dashboard/**: "Check for proper loading/error states."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/agents/agent-behavior-section.tsx` around lines
64 - 68, AgentBehaviorSection currently reads emoji via useAgentEmoji and falls
back to '' which renders an empty EmojiPickerButton while listAgentEmoji is
loading/fails; update AgentBehaviorSection to (1) read isLoading/isError from
useAgentEmoji and show a skeleton/placeholder component for EmojiPickerButton
while isLoading, and (2) provide a static unicode fallback for
DEFAULT_REACTION_ON_MESSAGE and DEFAULT_REACTION_ON_RESOLVED (lookup by name)
when unicodeMap.get(...) returns undefined or isError is true so the UI shows
meaningful emojis offline; locate logic around useAgentEmoji, unicodeMap,
DEFAULT_REACTION_ON_MESSAGE, DEFAULT_REACTION_ON_RESOLVED and the
EmojiPickerButton usage to implement these checks and fallbacks.
1 participant
Summary
WellKnownEmoji,EmojiValue,getEmoji(),DEFAULT_EMOJI_MAP) — the SDK is now the single source of truth for cross-platform emoji@IsWellKnownEmoji()DTO validator that lazily loads the SDK's emoji map to reject invalid emoji names at the API boundaryGET /agents/emojiendpoint returning the full list of supported emoji with unicode representations, so the dashboard doesn't hardcode emoji charactersonReactionhandler asReactionEvent(removesanycast), type config reaction fields asWellKnownEmoji | nullesmImporthelper toutils/esm-import.tsexecuteResolveSignal→resolveConversation(resolve is a first-class action, not a signal)Architecture
graph LR SDK["chat SDK<br/>WellKnownEmoji · EmojiValue · DEFAULT_EMOJI_MAP"] subgraph API CFG["AgentConfigResolver<br/>import type WellKnownEmoji"] DTO["AgentBehaviorDto<br/>@IsWellKnownEmoji()"] CS["ChatSdkService<br/>getEmoji() → EmojiValue"] IH["AgentInboundHandler<br/>EmojiValue on ReactionEvent"] EP["GET /agents/emoji<br/>returns DEFAULT_EMOJI_MAP"] VAL["IsWellKnownEmojiConstraint<br/>loads DEFAULT_EMOJI_MAP once"] end subgraph Dashboard BEH["AgentBehaviorSection<br/>fetches /agents/emoji"] end SDK -->|"type-only"| CFG SDK -->|"type-only"| IH SDK -->|"runtime via esmImport"| CS SDK -->|"runtime via esmImport"| EP SDK -->|"runtime via esmImport"| VAL EP -->|"HTTP"| BEH DTO --> VALWhat uses
EmojiValueat runtime vs. stringChatSdkService.reactToMessage/removeReactionEmojiValueviagetEmoji()ReactionEventhandlerEmojiValue(from SDK).nameextracted at boundaryResolvedAgentConfigWellKnownEmoji(string literal)stringTest plan
mockEmoji()helper forEmojiValuefixtures)PATCH /agents/:idwithbehavior.reactions.onMessageReceived: "not_real"returns 400PATCH /agents/:idwithbehavior.reactions.onMessageReceived: "fire"succeedsGET /agents/emojireturns list withnameandunicodefieldsMade with Cursor
What changed
Agent reactions now use the Chat SDK's type-safe emoji system instead of hardcoded emoji characters: configs reference WellKnownEmoji names, runtime code resolves to EmojiValue objects, and the API exposes supported emoji with unicode. This centralizes emoji as the single source of truth for cross-platform rendering, prevents invalid emoji config values, and lets the dashboard render platform-correct glyphs.
Affected areas
api: Added a new GET /agents/emoji endpoint and ListAgentEmoji use case; introduced @IsWellKnownEmoji() validator that lazily loads the Chat SDK map; agent config resolution and ChatSdkService now resolve names to EmojiValue at runtime and type-restrict reaction fields as WellKnownEmoji | null.
dashboard: Replaced hardcoded unicode in the agent behavior UI with a React Query hook that fetches emoji entries from /agents/emoji and maps names → unicode for rendering.
shared (agents utils): Added esmImport helper to dynamically import the Chat SDK at runtime and avoid circular/static import issues.
Key technical decisions
Testing
E2E tests updated to use mockEmoji() fixtures; validation behavior should be covered by tests for PATCH /agents/:id (reject invalid, accept valid); GET /agents/emoji returns name/unicode list for the dashboard. Manual verification: dashboard emoji picker and agent reaction flows.