feat(framework): add ctx.addReaction() to the agent SDK fixes NV-7411#10882
feat(framework): add ctx.addReaction() to the agent SDK fixes NV-7411#10882
Conversation
Adds addReaction(messageId, emojiName) to the AgentContext interface and AgentContextImpl. Reactions are queued and batched with the next reply() or flushed automatically when the handler completes — same contract as ctx.trigger(). Adds AddReactionPayload type and addReactions field to AgentReplyPayload (wire format). Made-with: Cursor
Adds a test confirming ctx.addReaction() without a subsequent reply() is flushed automatically when the handler completes, with no reply field in the outgoing body. Made-with: Cursor
Routes addReactions from the reply payload to ChatSdkService.reactToMessage() for each entry. Updates the "at least one of" guard to accept addReactions as a standalone operation. Wires the field through the webhook controller. Made-with: Cursor
Verifies that each addReaction entry in the payload routes to ChatSdkService.reactToMessage() with correct args, and that a payload with no recognised operation returns 400. Made-with: Cursor
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPropagates an Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as AgentContext
participant Webhook as API:/agents/:agentId/reply
participant UseCase as HandleAgentReplyUseCase
participant ChatSDK as ChatSdkService
Agent->>Agent: addReaction(messageId, emoji)
Agent->>Agent: reply(...) or flush()
Agent->>Webhook: POST /agents/:agentId/reply (payload includes addReactions)
Webhook->>UseCase: construct HandleAgentReplyCommand (addReactions)
UseCase->>ChatSDK: reactToMessage(messageId, emoji) for each addReactions entry
ChatSDK-->>UseCase: settled results
UseCase->>UseCase: resolveConversation / other ops
UseCase-->>Webhook: respond 200 / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
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/usecases/handle-agent-reply/handle-agent-reply.usecase.ts (1)
37-45:⚠️ Potential issue | 🟠 MajorValidation gap:
edit+addReactionsis silently accepted but reactions are dropped.Lines 59-61 return early on
command.edit, so theaddReactionsblock at lines 83-96 is unreachable on edit requests. Combined with the new line-43 guard (which now considersaddReactionsa valid standalone operation), anedit + addReactionspayload passes validation, runs the edit, and silently discards the reactions.Either reject the combination at the guard (consistent with the
edit + resolve/signalsrule), or processaddReactionson the edit path before returning. Rejecting is the safer/clearer fix.🛡️ Proposed fix — reject the combination
- if (command.edit && (command.resolve || command.signals?.length)) { - throw new BadRequestException('edit cannot be combined with resolve or signals'); + if (command.edit && (command.resolve || command.signals?.length || command.addReactions?.length)) { + throw new BadRequestException('edit cannot be combined with resolve, signals, or addReactions'); }🤖 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 37 - 45, The current validation allows command.edit together with command.addReactions, but the edit path returns early so reactions are silently dropped; update the validation in handle-agent-reply.usecase (the guard that currently checks command.edit && (command.resolve || command.signals?.length)) to also reject addReactions by changing it to check command.addReactions?.length and throw the same BadRequestException (i.e., make the condition: if (command.edit && (command.resolve || command.signals?.length || command.addReactions?.length)) throw new BadRequestException('edit cannot be combined with resolve or signals or addReactions')); this ensures edit+addReactions is rejected rather than processed and losing reactions.
🧹 Nitpick comments (4)
apps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.command.ts (1)
40-42: Consider reusingAddReactionPayloadDtofor the command field.
reply/editreuse their DTOs (ReplyContentDto,EditPayloadDto) with@ValidateNested+@Type, butaddReactionsfalls back to an inline type with only@IsArray. Validation already happens at the controller viaAgentReplyPayloadDto, so this is functionally equivalent today, but typing the field asAddReactionPayloadDto[]keeps the command shape aligned with the DTO and avoids drift if a field is added to the payload later.🤖 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.command.ts` around lines 40 - 42, The addReactions field in handle-agent-reply.command.ts should reuse the existing AddReactionPayloadDto instead of an inline type to keep the command shape aligned with DTOs; update the addReactions property to be AddReactionPayloadDto[] and apply `@ValidateNested`({ each: true }) and `@Type`(() => AddReactionPayloadDto) plus `@IsOptional`()/@IsArray() (matching how ReplyContentDto and EditPayloadDto are handled), so any future fields added to AddReactionPayloadDto are validated and typed consistently with AgentReplyPayloadDto.apps/api/src/app/agents/e2e/agent-reply.e2e.ts (2)
326-335: Test placement: this case isn't really aboutaddReactions.The 400 here exercises the global "at least one operation" guard with an entirely empty payload — it would behave identically without the
addReactionswork. Consider moving it to a top-level/Validationdescribe so theaddReactionsblock stays focused, or assert through a payload that's specifically empty-after-stripping reactions (e.g., explicitlyaddReactions: []) to actually exercise the new field.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/e2e/agent-reply.e2e.ts` around lines 326 - 335, The test 'should return 400 when payload has no recognised operation' is misplaced under the addReactions tests because it exercises the global "at least one operation" validation rather than addReactions specifically; either move this it-block out to a top-level or Validation describe to keep addReactions-focused tests scoped correctly, or change the request payload sent by postReply to include an empty addReactions array (e.g., addReactions: []) so the test actually exercises the new field's behavior while still asserting a 400; update references to seedConversation and postReply accordingly and keep the test name/expectation unchanged.
300-336: Missing coverage foredit+addReactionsinteraction.The use case currently returns early on
command.editbefore reaching theaddReactionsblock (seehandle-agent-reply.usecase.tslines 59-61 / 83-96), so a payload combiningeditwithaddReactionswill silently drop the reactions. Whichever way that ambiguity is resolved (reject at validation or process reactions on the edit path), please add an e2e case to lock the behavior in.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/e2e/agent-reply.e2e.ts` around lines 300 - 336, The new test should cover the interaction between command.edit and addReactions so we don't silently drop reactions; update apps/api/src/app/agents/e2e/agent-reply.e2e.ts to add an e2e case that posts a reply payload containing both command.edit and addReactions via postReply (use seedConversation and ctx.integrationIdentifier as in existing tests) and assert the expected behavior (either a 400 validation response if we decide to reject combined payloads, or that ChatSdkService.reactToMessage (reactToMessage stub) is called for each reaction even when command.edit is present). Ensure the test references the same helpers (postReply, seedConversation) and checks reactToMessage callCount and args to lock the chosen behavior; this will prevent regressions against handle-agent-reply.usecase.ts where command.edit currently returns early and skips addReactions.apps/api/src/app/agents/dtos/agent-reply-payload.dto.ts (1)
131-141: Consider validatingemojiNameagainst the supported set here.Currently
emojiNameonly requires a non-empty string at the DTO; an unknown name (e.g."thumsbup") will pass validation, hitChatSdkService.resolveEmoji, throwUnknown emoji name: …, and bubble up as a generic 500 instead of a clean 400 with the helpful "use GET /agents/emoji" hint. Validating at the DTO (or wrapping the resolveEmoji call to translate toBadRequestException) gives clients a much better failure mode. Optional — not blocking.🤖 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-reply-payload.dto.ts` around lines 131 - 141, Add validation for emojiName so unknown names return a 400 with a helpful message instead of a 500; implement either a class-validator constraint (e.g., create an injectable ValidatorConstraint like EmojiNameSupported and apply `@Validate`(EmojiNameSupported) to AddReactionPayloadDto.emojiName that checks the supported list from ChatSdkService or your emoji registry) or wrap ChatSdkService.resolveEmoji to catch the unknown-emoji error and rethrow a BadRequestException with guidance (e.g., "Unknown emoji name; see GET /agents/emoji"). Ensure the DTO uses the validator or the service wrapper so invalid emojiName values produce a BadRequestException rather than an internal error.
🤖 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/usecases/handle-agent-reply/handle-agent-reply.usecase.ts`:
- Around line 83-96: The current reaction batch uses Promise.all so a single
failed chatSdkService.reactToMessage (e.g., from resolveEmoji) will reject the
whole request and prevent the later command.resolve block from running; change
the block that handles command.addReactions to use Promise.allSettled instead of
Promise.all, iterate results to log any failures (include the reaction details
and error) but do not rethrow, making reactions best-effort; mirror the
fire-and-forget pattern used by removeAckReaction/reactOnResolve so the reply
persistence and the subsequent call to command.resolve still always run even if
some reactToMessage calls fail.
In `@packages/framework/src/resources/agent/agent.types.ts`:
- Around line 181-194: The PR adds a new public API (AgentContext.addReaction
and exported AddReactionPayload) but the framework package version was not
bumped; update packages/framework/package.json to a new minor pre-release (for
example from 2.10.1-alpha.2 to 2.11.0-alpha.0 or your repo's agreed pre-release
scheme) so the new exports are published, commit the version change and update
any changelog/release metadata as required.
---
Outside diff comments:
In
`@apps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.usecase.ts`:
- Around line 37-45: The current validation allows command.edit together with
command.addReactions, but the edit path returns early so reactions are silently
dropped; update the validation in handle-agent-reply.usecase (the guard that
currently checks command.edit && (command.resolve || command.signals?.length))
to also reject addReactions by changing it to check command.addReactions?.length
and throw the same BadRequestException (i.e., make the condition: if
(command.edit && (command.resolve || command.signals?.length ||
command.addReactions?.length)) throw new BadRequestException('edit cannot be
combined with resolve or signals or addReactions')); this ensures
edit+addReactions is rejected rather than processed and losing reactions.
---
Nitpick comments:
In `@apps/api/src/app/agents/dtos/agent-reply-payload.dto.ts`:
- Around line 131-141: Add validation for emojiName so unknown names return a
400 with a helpful message instead of a 500; implement either a class-validator
constraint (e.g., create an injectable ValidatorConstraint like
EmojiNameSupported and apply `@Validate`(EmojiNameSupported) to
AddReactionPayloadDto.emojiName that checks the supported list from
ChatSdkService or your emoji registry) or wrap ChatSdkService.resolveEmoji to
catch the unknown-emoji error and rethrow a BadRequestException with guidance
(e.g., "Unknown emoji name; see GET /agents/emoji"). Ensure the DTO uses the
validator or the service wrapper so invalid emojiName values produce a
BadRequestException rather than an internal error.
In `@apps/api/src/app/agents/e2e/agent-reply.e2e.ts`:
- Around line 326-335: The test 'should return 400 when payload has no
recognised operation' is misplaced under the addReactions tests because it
exercises the global "at least one operation" validation rather than
addReactions specifically; either move this it-block out to a top-level or
Validation describe to keep addReactions-focused tests scoped correctly, or
change the request payload sent by postReply to include an empty addReactions
array (e.g., addReactions: []) so the test actually exercises the new field's
behavior while still asserting a 400; update references to seedConversation and
postReply accordingly and keep the test name/expectation unchanged.
- Around line 300-336: The new test should cover the interaction between
command.edit and addReactions so we don't silently drop reactions; update
apps/api/src/app/agents/e2e/agent-reply.e2e.ts to add an e2e case that posts a
reply payload containing both command.edit and addReactions via postReply (use
seedConversation and ctx.integrationIdentifier as in existing tests) and assert
the expected behavior (either a 400 validation response if we decide to reject
combined payloads, or that ChatSdkService.reactToMessage (reactToMessage stub)
is called for each reaction even when command.edit is present). Ensure the test
references the same helpers (postReply, seedConversation) and checks
reactToMessage callCount and args to lock the chosen behavior; this will prevent
regressions against handle-agent-reply.usecase.ts where command.edit currently
returns early and skips addReactions.
In
`@apps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.command.ts`:
- Around line 40-42: The addReactions field in handle-agent-reply.command.ts
should reuse the existing AddReactionPayloadDto instead of an inline type to
keep the command shape aligned with DTOs; update the addReactions property to be
AddReactionPayloadDto[] and apply `@ValidateNested`({ each: true }) and `@Type`(()
=> AddReactionPayloadDto) plus `@IsOptional`()/@IsArray() (matching how
ReplyContentDto and EditPayloadDto are handled), so any future fields added to
AddReactionPayloadDto are validated and typed consistently with
AgentReplyPayloadDto.
🪄 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: d59e8a4d-32f3-47a1-843c-606008629e83
📒 Files selected for processing (8)
apps/api/src/app/agents/agents-webhook.controller.tsapps/api/src/app/agents/dtos/agent-reply-payload.dto.tsapps/api/src/app/agents/e2e/agent-reply.e2e.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.tspackages/framework/src/resources/agent/agent.context.tspackages/framework/src/resources/agent/agent.test.tspackages/framework/src/resources/agent/agent.types.ts
| /** | ||
| * Add an emoji reaction to any platform message. | ||
| * Reactions are queued and sent with the next `ctx.reply()`, or flushed automatically | ||
| * when the handler completes (same batching contract as `ctx.trigger()`). | ||
| * | ||
| * @param messageId - Platform-native message ID to react to (e.g. Slack `ts`). | ||
| * @param emojiName - Emoji short-name (e.g. `'thumbs_up'`, `'check_mark'`). | ||
| * | ||
| * @example | ||
| * ctx.addReaction(ctx.reaction!.messageId, 'check_mark'); | ||
| * await ctx.reply('Done!'); | ||
| */ | ||
| addReaction(messageId: string, emojiName: Emoji): void; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'package.json' packages/framework -d 2 --exec cat {} \; | head -40Repository: novuhq/novu
Length of output: 875
🏁 Script executed:
git diff HEAD packages/framework/package.json 2>/dev/null || echo "No diff found or git not available"Repository: novuhq/novu
Length of output: 37
🏁 Script executed:
# Also check if there were any version-related changes
git diff --no-ext-diff HEAD -- packages/framework/package.json | head -50Repository: novuhq/novu
Length of output: 37
🏁 Script executed:
# List the agent.types.ts file to see full exports
cat -n packages/framework/src/resources/agent/agent.types.ts | tail -30Repository: novuhq/novu
Length of output: 1041
Ensure @novu/framework version is bumped for new public exports.
The PR adds addReaction method to AgentContext and exports AddReactionPayload—both new public API surface additions to the framework SDK. Per the repo's semver convention for packages/** (new exports require a minor version bump), the version in packages/framework/package.json must be incremented. Currently no version bump is present in this PR.
Update the version from 2.10.1-alpha.2 to reflect the minor bump (e.g., 2.11.0-alpha.0 or appropriate pre-release) before merge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/framework/src/resources/agent/agent.types.ts` around lines 181 -
194, The PR adds a new public API (AgentContext.addReaction and exported
AddReactionPayload) but the framework package version was not bumped; update
packages/framework/package.json to a new minor pre-release (for example from
2.10.1-alpha.2 to 2.11.0-alpha.0 or your repo's agreed pre-release scheme) so
the new exports are published, commit the version change and update any
changelog/release metadata as required.
- Use AddReactionPayloadDto in HandleAgentReplyCommand instead of inline type, so @ValidateNested/@type decorators apply consistently - Add addReactions to the edit exclusivity guard so edit+addReactions returns 400 instead of silently dropping reactions - Switch Promise.all to Promise.allSettled so a failed reactToMessage does not block resolve or other operations; log failures as warnings - Add e2e test locking the edit+addReactions rejection behavior Made-with: Cursor
commit: |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.usecase.ts (1)
83-106: Reaction dispatch is appropriately best-effort.Switching to
Promise.allSettledwith per-failure warn logging cleanly addresses the prior concern: a bad emoji or adapter error no longer poisons the request after a successful reply, and the subsequentcommand.resolveblock on line 108 still runs. The log payload includes the offending reaction, which will be useful for triage.One small nit:
agentIdentifieris duplicated in the log fields and interpolated into the message — and the static message could carry the failing emoji to make grep-by-message more useful.♻️ Optional log tweak
- for (let i = 0; i < results.length; i++) { - const result = results[i]; - if (result.status === 'rejected') { - this.logger.warn( - { err: result.reason, reaction: command.addReactions[i], agentIdentifier: command.agentIdentifier }, - `[agent:${command.agentIdentifier}] Failed to add reaction` - ); - } - } + for (const [i, result] of results.entries()) { + if (result.status === 'rejected') { + const reaction = command.addReactions[i]; + this.logger.warn( + { err: result.reason, reaction, agentIdentifier: command.agentIdentifier }, + `[agent:${command.agentIdentifier}] Failed to add reaction "${reaction.emojiName}" to ${reaction.messageId}` + ); + } + }🤖 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 83 - 106, The warning log for failed reaction adds duplicates agentIdentifier in both the structured fields and the interpolated message and misses putting the failing emoji into the static message; update the warn call inside the addReactions handling (the block using Promise.allSettled and iterating results) to remove the duplicate agentIdentifier from the structured payload (keep it once), add the failing reaction/emoji (e.g., command.addReactions[i].emojiName or the full reaction object) into the structured fields, and include the emoji (or short reaction text) directly in the log message string so the message itself is grep-friendly while keeping full error details in the structured fields.
🤖 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/usecases/handle-agent-reply/handle-agent-reply.usecase.ts`:
- Around line 83-106: The warning log for failed reaction adds duplicates
agentIdentifier in both the structured fields and the interpolated message and
misses putting the failing emoji into the static message; update the warn call
inside the addReactions handling (the block using Promise.allSettled and
iterating results) to remove the duplicate agentIdentifier from the structured
payload (keep it once), add the failing reaction/emoji (e.g.,
command.addReactions[i].emojiName or the full reaction object) into the
structured fields, and include the emoji (or short reaction text) directly in
the log message string so the message itself is grep-friendly while keeping full
error details in the structured fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2c526a62-2b3e-43c5-a959-b05817bcb5e7
📒 Files selected for processing (3)
apps/api/src/app/agents/e2e/agent-reply.e2e.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.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/src/app/agents/e2e/agent-reply.e2e.ts
…411] Made-with: Cursor
…V-7411] Made-with: Cursor
What
Adds
ctx.addReaction(messageId, emojiName)toAgentContextso agent handler code can programmatically react to any platform message. Previously the SDK could only read incoming reactions (ctx.reaction) — there was no way to send one.Why
Agent handlers often need to acknowledge actions or signal state with emoji reactions (e.g.
eyeswhile thinking,checkwhen done). The backend capability (ChatSdkService.reactToMessage) already existed; this wires it up through the public SDK.How
Reactions follow the same batching contract as
ctx.trigger()— they are queued synchronously and flushed with the nextctx.reply(), or automatically when the handler completes.sequenceDiagram participant Handler as Agent handler participant Ctx as AgentContextImpl participant API as Novu API /reply participant Chat as ChatSdkService Handler->>Ctx: ctx.addReaction('msg-id', 'thumbs_up') Note over Ctx: queued in _pendingReactions[] Handler->>Ctx: await ctx.reply('Done!') Ctx->>API: POST {reply, addReactions: [{messageId, emojiName}]} API->>Chat: reactToMessage(..., 'msg-id', 'thumbs_up')If no
reply()is called, reactions are flushed automatically when the handler returns (same path asctx.trigger()-only handlers).Changes
agent.types.tsAddReactionPayloadinterface,addReactionsonAgentReplyPayload,addReaction()onAgentContext—emojiNametyped asEmojifromchatfor IDE autocompleteagent.context.ts_pendingReactionsqueue,addReaction()impl, drain inreply()andflush()agent-reply-payload.dto.tsAddReactionPayloadDto+addReactionsfield onAgentReplyPayloadDtohandle-agent-reply.usecase.tsPromise.alldispatch toreactToMessage, guard updatedagents-webhook.controller.tsaddReactionswired through to commandagent.test.tsagent-reply.e2e.tsTest plan
packages/framework: 28/28 unit tests passing (npx vitest run src/resources/agent/agent.test.ts)apps/api: 14/14 e2e tests passing (agent-reply.e2e.ts)Made with Cursor
What changed
The agent SDK now exposes ctx.addReaction(messageId, emojiName) so agent handlers can enqueue emoji reactions for platform messages; reactions are buffered and are sent with the next ctx.reply() or automatically flushed when the handler completes. The API was extended to accept addReactions on agent replies and to dispatch each reaction to ChatSdkService.reactToMessage, surfacing existing backend reaction support to public agent handlers.
Affected areas
framework: Added AgentContext.addReaction, an AddReactionPayload type, and a _pendingReactions buffer in AgentContextImpl; reply() and flush() include queued addReactions in outgoing AgentReplyPayloads and clear the buffer.
api: Added AddReactionPayloadDto and addReactions to AgentReplyPayloadDto and HandleAgentReplyCommand; the agents webhook controller forwards body.addReactions; the handle-agent-reply use case validates addReactions, rejects conflicting edit+resolve/signals/addReactions payloads, and dispatches each reaction to ChatSdkService.reactToMessage.
Key technical decisions
Testing
Added framework unit tests for flush-only reactions and reactions batched with reply; added API e2e tests for multi-reaction routing and rejection of edit+addReactions; reported runs: framework 28/28 passing, API e2e 14/14 passing.