Conversation
…fixes NV-7369 Add configurable emoji reactions so agents can automatically react to messages on the underlying chat platform (Slack, Teams, WhatsApp): - React to incoming messages with 👀 (eyes) by default - React to the final message when a conversation is resolved with ✅ (check) by default Both reactions are opt-out: set to null to disable, or provide a custom emoji name from the chat SDK's well-known set. Defaults are resolved at runtime (no migration needed). Also renames AgentCredentialService → AgentConfigResolver since it now resolves both credentials and behavior configuration. Made-with: Cursor
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
- Add createSentMessageFromMessage() + mockSentMessage to mockThread in agent-webhook.e2e.ts so inbound reaction calls don't throw - Stub chatSdkService.reactToMessage() in agent-reply.e2e.ts so resolve reaction doesn't attempt real credential resolution Made-with: Cursor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe PR implements agent message reaction capabilities by renaming Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Chat Client
participant API as Agent API
participant Handler as AgentInboundHandler
participant Repo as ConversationRepository
participant ChatSDK as ChatSdkService
participant Adapter as Platform Adapter
participant Platform as Slack/External
Client->>API: Send message to thread
API->>Handler: handle(message, config)
Handler->>Repo: Check firstPlatformMessageId
alt First Message Detected & reactionOnMessageReceived Set
Handler->>ChatSDK: reactToMessage(platformMessageId, emoji)
ChatSDK->>Adapter: addReaction(platformMessageId, emoji)
Adapter->>Platform: Add emoji reaction
Platform-->>Adapter: ✓
Adapter-->>ChatSDK: ✓
ChatSDK-->>Handler: ✓
Handler->>Repo: setFirstPlatformMessageId(messageId)
Repo-->>Handler: ✓
end
Handler-->>API: ✓
rect rgba(0, 150, 255, 0.5)
Note over Handler,Platform: Reaction flow on resolve
API->>Handler: On agent resolve signal
Handler->>ChatSDK: reactToMessage(firstPlatformMessageId, 'check')
ChatSDK->>Adapter: addReaction(firstPlatformMessageId, emoji)
Adapter->>Platform: Add check reaction
Platform-->>Adapter: ✓
Adapter-->>ChatSDK: ✓
end
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
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/chat-sdk.service.ts (1)
131-150:⚠️ Potential issue | 🟠 MajorAdd cache eviction to all agent write paths that modify behavior or integration linkage.
registerEventHandlers()captures aResolvedAgentConfigsnapshot when theChatinstance is created, andgetOrCreate()reuses that instance for up to 30 minutes. Updates to agent behavior (thinkingIndicatorEnabled, reactions) or agent-integration linkage bypass cache invalidation entirely, leaving inbound webhooks to consume stale configuration until the TTL expires.Add
chatSdkService.evict()calls to:
UpdateAgent.execute()whenbehavioris modified (after database update)UpdateAgentIntegration.execute()when integration is changed (after database update)DeleteAgent.execute()when agent is deleted (after database delete)This requires injecting
ChatSdkServiceinto these usecases. Alternatively, move eviction logic to the controller layer after each usecase succeeds.Also applies to: 239-243
🤖 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 131 - 150, getOrCreate currently returns long-lived Chat instances built from the ResolvedAgentConfig captured in registerEventHandlers, causing stale behavior when agent settings or integrations change; after the DB updates in UpdateAgent.execute (when behavior fields like thinkingIndicatorEnabled or reactions change), UpdateAgentIntegration.execute (when integration linkage changes), and DeleteAgent.execute (after agent deletion), call ChatSdkService.evict(instanceKey) to remove cached Chat instances so they will be rebuilt with fresh config; inject ChatSdkService into those usecase classes (or call eviction from the controller after each successful usecase) and ensure the instanceKey you pass matches the key used by ChatSdkService.getOrCreate so inbound webhooks pick up the new configuration 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/dtos/agent-behavior.dto.ts`:
- Around line 17-20: The onResolved property currently has `@IsString`() which
rejects null; add a `@ValidateIf`((_, value) => value !== null) decorator before
`@IsString`() (keep `@IsOptional`()) on the onResolved field in the AgentBehaviorDto
so null is allowed but non-null values are still validated as strings.
- Around line 10-12: The `@IsString`() validator rejects null, breaking the
documented opt-out; update the DTO so null bypasses string validation by adding
a ValidateIf check: add `@ValidateIf`(o => o.onMessageReceived !== null) above the
onMessageReceived property and do the same (`@ValidateIf`(o => o.onResolved !==
null)) above onResolved, keeping `@IsOptional`() and `@IsString`() to validate only
when the value is not null.
In
`@apps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.usecase.ts`:
- Around line 222-234: The reaction is being sent using channel.platformThreadId
but the code already fetched the inbound activity via findLastInboundMessage (in
handle-agent-reply.usecase.ts), which returns lastInbound.platformThreadId;
change the reactToMessage call in the block that uses lastInbound to pass
lastInbound.platformThreadId (or fallback to channel.platformThreadId if
lastInbound.platformThreadId is missing) so the reaction targets the actual
inbound message thread; update the call site of chatSdkService.reactToMessage
accordingly.
---
Outside diff comments:
In `@apps/api/src/app/agents/services/chat-sdk.service.ts`:
- Around line 131-150: getOrCreate currently returns long-lived Chat instances
built from the ResolvedAgentConfig captured in registerEventHandlers, causing
stale behavior when agent settings or integrations change; after the DB updates
in UpdateAgent.execute (when behavior fields like thinkingIndicatorEnabled or
reactions change), UpdateAgentIntegration.execute (when integration linkage
changes), and DeleteAgent.execute (after agent deletion), call
ChatSdkService.evict(instanceKey) to remove cached Chat instances so they will
be rebuilt with fresh config; inject ChatSdkService into those usecase classes
(or call eviction from the controller after each successful usecase) and ensure
the instanceKey you pass matches the key used by ChatSdkService.getOrCreate so
inbound webhooks pick up the new configuration 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: 85ba1851-e5d7-4276-8156-e03875eea7ba
📒 Files selected for processing (14)
apps/api/src/app/agents/agents.module.tsapps/api/src/app/agents/dtos/agent-behavior.dto.tsapps/api/src/app/agents/e2e/agent-reply.e2e.tsapps/api/src/app/agents/e2e/agent-webhook.e2e.tsapps/api/src/app/agents/e2e/agents.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/bridge-executor.service.tsapps/api/src/app/agents/services/chat-sdk.service.tsapps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.usecase.tsapps/api/src/app/agents/usecases/update-agent/update-agent.usecase.tslibs/dal/src/repositories/agent/agent.entity.tslibs/dal/src/repositories/agent/agent.schema.tslibs/dal/src/repositories/conversation-activity/conversation-activity.repository.ts
| const lastInbound = await this.activityRepository.findLastInboundMessage( | ||
| command.environmentId, | ||
| conversation._id | ||
| ); | ||
| if (!lastInbound?.platformMessageId) return; | ||
|
|
||
| await this.chatSdkService.reactToMessage( | ||
| conversation._agentId, | ||
| command.integrationIdentifier, | ||
| channel.platform, | ||
| channel.platformThreadId, | ||
| lastInbound.platformMessageId, | ||
| config.reactionOnResolved |
There was a problem hiding this comment.
Use the inbound activity's thread ID for the resolve reaction.
findLastInboundMessage() already returns platformThreadId, but this call still uses channel.platformThreadId from channels[0]. If the last inbound message is tied to a different thread, the reaction will be sent to the wrong target or fail.
Proposed fix
await this.chatSdkService.reactToMessage(
conversation._agentId,
command.integrationIdentifier,
channel.platform,
- channel.platformThreadId,
+ lastInbound.platformThreadId,
lastInbound.platformMessageId,
config.reactionOnResolved
);📝 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 lastInbound = await this.activityRepository.findLastInboundMessage( | |
| command.environmentId, | |
| conversation._id | |
| ); | |
| if (!lastInbound?.platformMessageId) return; | |
| await this.chatSdkService.reactToMessage( | |
| conversation._agentId, | |
| command.integrationIdentifier, | |
| channel.platform, | |
| channel.platformThreadId, | |
| lastInbound.platformMessageId, | |
| config.reactionOnResolved | |
| const lastInbound = await this.activityRepository.findLastInboundMessage( | |
| command.environmentId, | |
| conversation._id | |
| ); | |
| if (!lastInbound?.platformMessageId) return; | |
| await this.chatSdkService.reactToMessage( | |
| conversation._agentId, | |
| command.integrationIdentifier, | |
| channel.platform, | |
| lastInbound.platformThreadId, | |
| lastInbound.platformMessageId, | |
| config.reactionOnResolved |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.usecase.ts`
around lines 222 - 234, The reaction is being sent using
channel.platformThreadId but the code already fetched the inbound activity via
findLastInboundMessage (in handle-agent-reply.usecase.ts), which returns
lastInbound.platformThreadId; change the reactToMessage call in the block that
uses lastInbound to pass lastInbound.platformThreadId (or fallback to
channel.platformThreadId if lastInbound.platformThreadId is missing) so the
reaction targets the actual inbound message thread; update the call site of
chatSdkService.reactToMessage accordingly.
…emove on reply, resolve on thread starter - Only react with 👀 on the first inbound message that starts a conversation (not on every follow-up message in the thread) - Remove the 👀 reaction when the bot sends its first reply - Place the ✅ resolve reaction on the thread-starting message (not the last user message) — matches Cursor/Linear industry pattern - Store firstPlatformMessageId on ConversationChannel for both reaction removal and resolve targeting - Add removeReaction() to ChatSdkService - Remove findLastInboundMessage() from ConversationActivityRepository (no longer needed — we use firstPlatformMessageId instead) Made-with: Cursor
Keep our AgentConfigResolver rename and reaction lifecycle while accepting next's ReplyContentDto-based deliverMessage signature and rich content support. Made-with: Cursor
Add @ValidateIf to skip @IsString when value is null, so setting onMessageReceived: null or onResolved: null correctly disables reactions instead of failing validation. Made-with: Cursor
3b3ffb7 to
899aa88
Compare
Summary
eyesby default) and to the last message when a conversation is resolved (✅checkby default)nullto disable, or provide a custom emoji name from thechatSDK's well-known set (80+ cross-platform emoji)AgentEntity.behavior.reactions— no migration neededAgentCredentialService→AgentConfigResolver(andResolvedPlatformConfig→ResolvedAgentConfig) since it now resolves both credentials and behavior configurationHow it works
sequenceDiagram participant User as User (Slack/Teams) participant IH as AgentInboundHandler participant Thread as Thread (chat SDK) participant HAR as HandleAgentReply Note over User, HAR: Inbound — react on message received User->>IH: sends message IH->>IH: persistInboundMessage() IH-)Thread: sentMessage.addReaction("eyes" 👀) [fire-and-forget] Thread-)User: 👀 appears on message Note over User, HAR: Resolve — react on conversation resolved HAR->>HAR: updateStatus(RESOLVED) HAR->>HAR: find last inbound platformMessageId HAR-)Thread: adapter.addReaction("check" ✅) [fire-and-forget] Thread-)User: ✅ appears on last messageChanged files (13)
agent.entity.ts,agent.schema.ts,conversation-activity.repository.tsagent-behavior.dto.tsagent-config-resolver.service.ts(renamed fromagent-credential.service.ts)agent-inbound-handler.service.ts,chat-sdk.service.ts,handle-agent-reply.usecase.tsupdate-agent.usecase.tsagents.module.ts,bridge-executor.service.tsagents.e2e.ts,agent-webhook.e2e.tsTest plan
null, verify partial updatesnull), verify no reaction firesFixes NV-7369
Made with Cursor
Note
Medium Risk
Adds new persisted agent behavior settings and triggers side-effecting reaction calls through the chat adapter on inbound messages and on resolve. While scoped, it touches conversation lifecycle paths and relies on platform reaction APIs and new defaults/null semantics.
Overview
Adds configurable emoji reactions to agent conversations: agents can auto-react on inbound messages and when a conversation is resolved (defaults:
eyeson message,checkon resolve;nulldisables).Extends agent behavior DTOs and DAL schema/entity to persist
behavior.reactions, updates the agent PATCH use case to support partial reaction updates, and introducesAgentConfigResolver(renaming/expanding the prior credential resolver) to resolve credentials plus behavior defaults at runtime.Wires reactions into runtime flows:
AgentInboundHandlerfire-and-forgetsaddReactionon receipt,HandleAgentReplyreacts on resolve using the last inbound message (newConversationActivityRepository.findLastInboundMessage), andChatSdkServiceaddsreactToMessage; E2E tests cover reaction CRUD/default/disable behavior.Reviewed by Cursor Bugbot for commit 40b9dd3. Configure here.
What changed
Agents now support configurable emoji reactions to conversations. They automatically react to incoming messages and when conversations resolve, using defaults ("eyes" for inbound, "check" for resolution) that can be customized or disabled. The service layer was refactored to clarify concerns:
AgentCredentialService→AgentConfigResolverandResolvedPlatformConfig→ResolvedAgentConfig, reflecting that the service now resolves both credentials and behavior configuration.Affected areas
api: Renamed
AgentCredentialServicetoAgentConfigResolverand updated its resolved config interface to includereactionOnMessageReceivedandreactionOnResolvedfields with computed defaults. Added reaction DTO validation (AgentReactionSettingsDtowith nullable string fields). ExtendedAgentInboundHandlerto detect first messages and fire-and-forget reaction additions; wiredChatSdkService.reactToMessage()andremoveReaction()to call underlying adapters. UpdatedHandleAgentReplyto remove the "ack" reaction after bot reply and add the "check" reaction on conversation resolve. EnhancedUpdateAgentuse case to persist partial reaction updates. E2E tests extended with reaction method stubs and new test case validating reaction CRUD.dal: Added
AgentReactionSettingsinterface andreactionsproperty toAgentBehavior. ExtendedConversationChannelwithfirstPlatformMessageIdto track the thread-starting message. AddedsetFirstPlatformMessageId()repository method for atomic updates.Key technical decisions
Testing
E2E tests added for reaction CRUD (agent patching reactions settings) and for the inbound/resolve reaction flow (stubs for
ChatSdkServicemethods). Manual verification steps documented in PR. Some reaction CRUD tests remain pending.