Conversation
Add App Secret and Verify Token to whatsAppBusinessConfig for agent webhook verification. Fix ChatSdkService credential mapping so apiToken → accessToken aligns with the outbound WhatsAppBusinessHandler. Made-with: Cursor
Meta requires a GET endpoint to verify webhook ownership during setup. Both GET and POST delegate to the same ChatSdkService.handleWebhook() since @chat-adapter/whatsapp handles both verification and event delivery. Made-with: Cursor
Remove comingSoon flag from WhatsApp Business in conversational providers. Add PLATFORMS_WITHOUT_TYPING_INDICATOR set and skip typing indicator for platforms that don't support it (WhatsApp). Made-with: Cursor
…idge pipeline Inbound media from chat platforms (images, documents, audio, video) was silently dropped — the pipeline only passed message.text. This wires up the full passthrough: Chat SDK Message.attachments → persist as richContent on ConversationActivity → include in bridge payload and history replay → expose as ctx.message.attachments in the framework. Made-with: Cursor
✅ Deploy preview added
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
bot
changed the title
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCentralized webhook handling added (GET verification + POST inbound) and delegated to a new helper; richContent/attachments support propagated through agent types, bridge, inbound handling, conversation service, and repository; WhatsApp credential mapping and runtime validation updated; typing indicator gated per-platform set. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WebhookController
participant ChatSdkService
participant AgentInboundHandler
participant ConversationService
participant Repo
Client->>WebhookController: GET/POST /:agentId/webhook/:integrationIdentifier
WebhookController->>WebhookController: routeWebhook(...)
WebhookController->>ChatSdkService: handleWebhook(req, res, agentId, integrationIdentifier)
ChatSdkService->>AgentInboundHandler: dispatch inbound event
AgentInboundHandler->>ConversationService: persistInboundMessage(params with richContent)
ConversationService->>Repo: createUserActivity({..., richContent})
Repo-->>ConversationService: activity persisted
ConversationService-->>AgentInboundHandler: ack
AgentInboundHandler-->>ChatSdkService: processing complete
ChatSdkService-->>WebhookController: result
WebhookController-->>Client: HTTP response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/agents-webhook.controller.ts (1)
81-90:⚠️ Potential issue | 🟠 MajorHarden webhook error handling with logging and
headersSentguard.Line 88 returns a generic 500 but the error is not logged, and there’s no guard for already-sent headers before writing a fallback response.
💡 Suggested patch
import { Body, Controller, Get, HttpCode, HttpException, HttpStatus, Param, Post, Req, Res, UseGuards, } from '@nestjs/common'; +import { PinoLogger } from '@novu/application-generic'; @@ constructor( private chatSdkService: ChatSdkService, - private handleAgentReplyUsecase: HandleAgentReply + private handleAgentReplyUsecase: HandleAgentReply, + private readonly logger: PinoLogger ) {} @@ - private async routeWebhook(agentId: string, integrationIdentifier: string, req: Request, res: Response) { + private async routeWebhook(agentId: string, integrationIdentifier: string, req: Request, res: Response): Promise<void> { try { await this.chatSdkService.handleWebhook(agentId, integrationIdentifier, req, res); } catch (err) { + this.logger.error(err, `Webhook handling failed for agent=${agentId}, integration=${integrationIdentifier}`); + + if (res.headersSent) { + return; + } + if (err instanceof HttpException) { res.status(err.getStatus()).json(err.getResponse()); + + return; } else { res.status(HttpStatus.INTERNAL_SERVER_ERROR).json({ error: 'Internal server error' }); } } }As per coding guidelines,
apps/api/**: “Check for proper error handling and input validation.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/agents-webhook.controller.ts` around lines 81 - 90, The routeWebhook error handler for routeWebhook (which calls chatSdkService.handleWebhook) must log unexpected errors and avoid writing a fallback response if the response is already sent; update the catch block to (1) log the full error (e.g., this.logger.error or processLogger.error) including stack/message when err is not an HttpException, and (2) before calling res.status(...).json(...) check res.headersSent and only attempt to write the fallback 500 if headersSent is false; keep existing HttpException handling intact.
🧹 Nitpick comments (1)
apps/api/src/app/agents/services/bridge-executor.service.ts (1)
307-314: Consider extracting a shared attachment mapper to avoid schema drift.The attachment projection here matches
apps/api/src/app/agents/services/agent-inbound-handler.service.ts(Line 93-103). Centralizing this shape in one mapper/type would reduce divergence risk as fields evolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/services/bridge-executor.service.ts` around lines 307 - 314, The attachment mapping in bridge-executor.service.ts (the mapped.attachments = message.attachments.map(...) block) duplicates the projection used in agent-inbound-handler.service.ts; extract a single shared mapper and/or type (e.g., mapAttachment and AttachmentDto) into a common module and replace both inline mappings with calls to that function to prevent schema drift—update usages in BridgeExecutorService (mapped.attachments) and AgentInboundHandlerService to import and use the shared mapAttachment/AttachmentDto so any field changes are made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/shared/src/consts/providers/credentials/provider-credentials.ts`:
- Around line 1293-1306: The credential entries for CredentialsKeyEnum.SecretKey
and CredentialsKeyEnum.Token are marked required: false but downstream code
dereferences credentials.secretKey! and credentials.token!, so update both
entries to required: true (change the required flag on the objects keyed by
CredentialsKeyEnum.SecretKey and CredentialsKeyEnum.Token) to enforce validation
at save-time; additionally, audit the adapter setup code that uses
credentials.secretKey! and credentials.token! (remove non-null assertions or add
runtime checks) so runtime dereferences are safe if inputs ever bypass schema
validation.
---
Outside diff comments:
In `@apps/api/src/app/agents/agents-webhook.controller.ts`:
- Around line 81-90: The routeWebhook error handler for routeWebhook (which
calls chatSdkService.handleWebhook) must log unexpected errors and avoid writing
a fallback response if the response is already sent; update the catch block to
(1) log the full error (e.g., this.logger.error or processLogger.error)
including stack/message when err is not an HttpException, and (2) before calling
res.status(...).json(...) check res.headersSent and only attempt to write the
fallback 500 if headersSent is false; keep existing HttpException handling
intact.
---
Nitpick comments:
In `@apps/api/src/app/agents/services/bridge-executor.service.ts`:
- Around line 307-314: The attachment mapping in bridge-executor.service.ts (the
mapped.attachments = message.attachments.map(...) block) duplicates the
projection used in agent-inbound-handler.service.ts; extract a single shared
mapper and/or type (e.g., mapAttachment and AttachmentDto) into a common module
and replace both inline mappings with calls to that function to prevent schema
drift—update usages in BridgeExecutorService (mapped.attachments) and
AgentInboundHandlerService to import and use the shared
mapAttachment/AttachmentDto so any field changes are made in one place.
🪄 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: 9f0ef403-20cf-42fc-a69b-db14ad3da141
📒 Files selected for processing (11)
apps/api/src/app/agents/agents-webhook.controller.tsapps/api/src/app/agents/dtos/agent-platform.enum.tsapps/api/src/app/agents/services/agent-conversation.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.tslibs/dal/src/repositories/conversation-activity/conversation-activity.repository.tspackages/framework/src/resources/agent/agent.types.tspackages/framework/src/resources/agent/index.tspackages/shared/src/consts/providers/conversational-providers.tspackages/shared/src/consts/providers/credentials/provider-credentials.ts
Comment on lines
+1293
to
+1306
| { | ||
| key: CredentialsKeyEnum.SecretKey, | ||
| displayName: 'App Secret', | ||
| description: 'Found under App Settings > Basic in your Meta app dashboard — used to verify inbound webhook signatures', | ||
| type: 'string', | ||
| required: false, | ||
| }, | ||
| { | ||
| key: CredentialsKeyEnum.Token, | ||
| displayName: 'Verify Token', | ||
| description: 'A secret string you define — must match the Verify Token entered in your Meta webhook configuration', | ||
| type: 'string', | ||
| required: false, | ||
| }, |
Contributor
There was a problem hiding this comment.
App Secret and Verify Token should not be optional if runtime treats them as required.
On Lines 1298 and 1305 these fields are optional, but downstream adapter setup dereferences credentials.secretKey! and credentials.token!. That allows invalid configs at save-time and fails later at runtime.
Suggested fix
{
key: CredentialsKeyEnum.SecretKey,
displayName: 'App Secret',
description: 'Found under App Settings > Basic in your Meta app dashboard — used to verify inbound webhook signatures',
type: 'string',
- required: false,
+ required: true,
},
{
key: CredentialsKeyEnum.Token,
displayName: 'Verify Token',
description: 'A secret string you define — must match the Verify Token entered in your Meta webhook configuration',
type: 'string',
- required: false,
+ required: true,
},📝 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.
Suggested change
| { | |
| key: CredentialsKeyEnum.SecretKey, | |
| displayName: 'App Secret', | |
| description: 'Found under App Settings > Basic in your Meta app dashboard — used to verify inbound webhook signatures', | |
| type: 'string', | |
| required: false, | |
| }, | |
| { | |
| key: CredentialsKeyEnum.Token, | |
| displayName: 'Verify Token', | |
| description: 'A secret string you define — must match the Verify Token entered in your Meta webhook configuration', | |
| type: 'string', | |
| required: false, | |
| }, | |
| { | |
| key: CredentialsKeyEnum.SecretKey, | |
| displayName: 'App Secret', | |
| description: 'Found under App Settings > Basic in your Meta app dashboard — used to verify inbound webhook signatures', | |
| type: 'string', | |
| required: true, | |
| }, | |
| { | |
| key: CredentialsKeyEnum.Token, | |
| displayName: 'Verify Token', | |
| description: 'A secret string you define — must match the Verify Token entered in your Meta webhook configuration', | |
| type: 'string', | |
| required: true, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/src/consts/providers/credentials/provider-credentials.ts`
around lines 1293 - 1306, The credential entries for
CredentialsKeyEnum.SecretKey and CredentialsKeyEnum.Token are marked required:
false but downstream code dereferences credentials.secretKey! and
credentials.token!, so update both entries to required: true (change the
required flag on the objects keyed by CredentialsKeyEnum.SecretKey and
CredentialsKeyEnum.Token) to enforce validation at save-time; additionally,
audit the adapter setup code that uses credentials.secretKey! and
credentials.token! (remove non-null assertions or add runtime checks) so runtime
dereferences are safe if inputs ever bypass schema validation.
commit: |
ChmaraX
force-pushed
the
nv-7374-add-whatsapp-support-for-agent-conversations
branch
from
3c28891 to
ab3ebcb
Compare
Replace non-null assertions with explicit guards in buildAdapters() for Slack, Teams, and WhatsApp. Fails fast with a clear error message when credentials are missing instead of passing undefined to the adapter. Made-with: Cursor
ChmaraX
deleted the
nv-7374-add-whatsapp-support-for-agent-conversations
branch
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds full backend support for WhatsApp as an agent conversation platform, enabling bidirectional messaging between WhatsApp users and Novu agent handlers.
What changed:
whatsAppBusinessConfigwithAppSecretandVerifyTokenfields; fixed the adapter credential mapping inChatSdkService(apiToken→accessToken,token→verifyToken)GETendpoint on the webhook controller for Meta's webhook URL verification handshake (delegates to the samerouteWebhookasPOST)PLATFORMS_WITHOUT_TYPING_INDICATORset to generically suppress typing indicators on platforms that don't support them (WhatsApp); confirmedctx.update()is safe (posts new message, not an edit)comingSoon: truefromCONVERSATIONAL_PROVIDERSso WhatsApp is selectableMessage.attachmentsthrough the full pipeline: persisted asrichContentonConversationActivity, included in bridge payload asmessage.attachments, replayed inhistory[].richContent, exposed asAgentAttachmentin the framework SDKAdapter-level limitations (not in scope):
@chat-adapter/whatsapp(files silently ignored)Select/list interactive messages not mapped by the adapter (falls back to text)sequenceDiagram participant WA as WhatsApp User participant API as Novu API participant SDK as Chat SDK Adapter participant Bridge as Customer Handler WA->>API: POST webhook (message + media) API->>SDK: handleWebhook() SDK->>API: onMessage(thread, message) API->>API: persist activity + richContent API->>Bridge: POST bridge payload (text + attachments) Bridge->>API: ctx.reply(text / markdown / card) API->>SDK: thread.post(content) SDK->>WA: Deliver messageTest plan
ctx.message.textCard+Buttonrenders as WhatsApp interactive reply buttonsonActionwith correctactionId/valuectx.message.attachmentspopulated with type/url/mimeTyperichContentand replayed inctx.historyMade with Cursor
What changed
WhatsApp Business is added as a supported agent conversation platform: webhook verification (GET) and inbound message POSTs are routed to a unified handler, WhatsApp credentials and validation were updated (App Secret & Verify Token added; apiToken → accessToken, token → verifyToken), inbound media attachments are passed through and persisted as richContent, and WhatsApp was enabled in the conversational provider list. This enables bidirectional messaging between WhatsApp users and Novu agent handlers and preserves history/replay of inbound media.
Affected areas
api: Added GET webhook verification endpoint and refactored webhook routing into a shared routeWebhook handler; added PLATFORMS_WITHOUT_TYPING_INDICATOR and conditional typing behavior; updated inbound handler to extract attachments and persist richContent; and fixed WhatsApp credential mapping and runtime validation in chat SDK adapter construction.
dal: ConversationActivityRepository.createUserActivity now accepts and persists an optional richContent field for user activities.
shared: Removed comingSoon flag for WhatsApp Business in CONVERSATIONAL_PROVIDERS and extended whatsAppBusinessConfig with optional App Secret and Verify Token credential entries.
framework: Exposed AgentAttachment type and added attachments to AgentMessage and richContent to AgentHistoryEntry so framework consumers can access inbound media and history entries.
providers/worker/dashboard: Existing provider/worker logic and UI entries now include WhatsApp where applicable (configuration and provider lists).
Key technical decisions
Testing
Manual verification performed: webhook verification via local tunnel, inbound text delivered correctly, outbound plain text/markdown/buttons and callbacks tested; inbound image/history replay noted as pending completion. No new automated tests were added in this PR.