feat(root): add @novu/chat-sdk-adapter Chat SDK platform adapter fixes NV-8063#11593
Conversation
…-8063 Introduce @chat-adapter/novu (packages/chat-adapter), a Chat SDK platform adapter that exposes Novu's normalized channels (Slack, WhatsApp, Teams, Telegram, Email) as one platform. The developer's Chat SDK app becomes the Novu bridge: verify inbound HMAC, map AgentBridgeRequest to Thread/Message, route to native handlers, and POST AgentReplyPayload to /v1/agents/:id/reply. - Inbound: raw-body HMAC verify, server-truth routing, deliveryId dedup, onResolve no-op - Outbound: reply/edit/addReactions POSTs (apiKey auth), edit-based streaming - Security: reply URL derived from config; request replyUrl ignored - Bundled zero-deps in-memory state; opt-in getNovuContext (trigger/metadata/resolve) - Next.js playground example (live bridge + credential-free simulator + test UI)
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
|
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:
📝 WalkthroughWalkthroughIntroduces the new Sequence DiagramssequenceDiagram
participant NovuBridge
participant NovuAdapterImpl
participant WebhookHandler
participant StateAdapter
participant Chat
participant ReplyClient
NovuBridge->>NovuAdapterImpl: handleWebhook(request)
NovuAdapterImpl->>WebhookHandler: parseAndVerify(request)
WebhookHandler-->>NovuAdapterImpl: ParseResult {status, AgentBridgeRequest?}
NovuAdapterImpl->>StateAdapter: setIfNotExists(deliveryId TTL key)
StateAdapter-->>NovuAdapterImpl: deduplicated?
NovuAdapterImpl->>StateAdapter: set(thread snapshot, TTL 7d)
NovuAdapterImpl->>Chat: processMessage / processAction / processReaction
Chat-->>NovuAdapterImpl: handler reply content
NovuAdapterImpl->>ReplyClient: send(AgentReplyPayload)
ReplyClient->>NovuBridge: POST /v1/agent/reply (ApiKey auth)
ReplyClient-->>NovuAdapterImpl: SentMessageInfo or null
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
| return timingSafeEqual(Buffer.from(receivedHmac, 'hex'), Buffer.from(expectedHmac, 'hex')); | ||
| } | ||
|
|
||
| export function getSignatureHeader(request: Request): string | null { |
There was a problem hiding this comment.
🔒 Agentic Security Review
Severity: MEDIUM
The signature verifier can throw on malformed novu-signature values: timingSafeEqual(Buffer.from(receivedHmac, 'hex'), ...) is called after checking only the hex-string length, but invalid hex can decode to a shorter buffer and cause timingSafeEqual to raise.
Impact: unauthenticated callers can reliably trigger 500 responses on the webhook verification path instead of a clean 401 reject, creating an avoidable availability/abuse vector on a public endpoint.
Reviewed by Cursor Security Reviewer for commit 3fd05cd. Configure here.
There was a problem hiding this comment.
Stale comment
Risk: medium. Not approving: Cursor Security Agent reported an unresolved medium-severity finding in webhook signature verification (malformed hex can throw and return 500s instead of 401). Assigned ChmaraX and djabarovgeorge for human review.
Sent by Cursor Approval Agent: Pull Request Approver
Replace @chat-adapter/novu with @novu/chat-sdk-adapter across package metadata, playground imports, and lockfile.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
playground/nextjs/src/app/novu-agent/page.tsx (1)
13-13: 💤 Low valueConsider using a named export in addition to the default export.
The coding guidelines prefer named exports for all components. While Next.js App Router pages require a default export, you can satisfy both requirements by using a named export and then re-exporting it as default.
♻️ Optional refactor pattern
-export default function NovuAgentPlayground() { +export function NovuAgentPlayground() { // ... component body ... } + +export default NovuAgentPlayground;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@playground/nextjs/src/app/novu-agent/page.tsx` at line 13, The NovuAgentPlayground component currently uses only a default export, but the coding guidelines prefer named exports. Refactor the component by first defining it as a named export function NovuAgentPlayground, then create a separate default export statement that re-exports the named export. This satisfies both the coding guideline requirement for named exports and the Next.js App Router requirement for a default export on page components.Source: Coding guidelines
packages/chat-adapter/src/thread-id.spec.ts (1)
28-32: ⚡ Quick winConsider adding test coverage for empty platform and invalid DM flag.
The malformed id tests don't cover cases like
'novu::int:conv:0'(empty platform) or'novu:slack:int:conv:X'(invalid DM flag). Adding these would strengthen validation and align with the suggested improvement inthread-id.tsline 26.📋 Suggested additional test cases
it('throws on malformed ids', () => { - for (const bad of ['', 'novu', 'novu:slack', 'email:foo:bar', 'novu:slack:int::0']) { + for (const bad of [ + '', + 'novu', + 'novu:slack', + 'email:foo:bar', + 'novu:slack:int::0', + 'novu::int:conv:0', + 'novu:slack:int:conv:X', + ]) { expect(() => decodeThreadId(bad)).toThrow(); } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/chat-adapter/src/thread-id.spec.ts` around lines 28 - 32, The test for malformed ids in the it block labeled 'throws on malformed ids' does not include test cases for empty platforms or invalid DM flag values. Add the two missing test cases to the bad array that is iterated over in the for loop: one case with an empty platform segment (novu::int:conv:0) and one case with an invalid DM flag value (novu:slack:int:conv:X). This will ensure that the decodeThreadId function properly validates these edge cases as required by the validation logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/chat-adapter/package.json`:
- Line 13: The `afterinstall` script in the scripts section of package.json is
not a recognized npm lifecycle hook and will not execute automatically during
installation. Rename the `afterinstall` script to `prepare`, which is a standard
npm lifecycle hook that will run automatically before pack/publish operations
and during local install, ensuring the build command executes when needed.
In `@packages/chat-adapter/src/adapter.ts`:
- Around line 132-166: The dedupe marker is being set via state.setIfNotExists
with deliveryKey before the actual message processing occurs. Move the dedupe
marker logic to execute after all dispatching operations (dispatchMessage,
dispatchAction, dispatchReaction) and the subscription setup complete
successfully. This ensures that if any of the dispatch or processing steps fail,
the delivery will not be marked as deduped and can be retried on the next
attempt without being treated as a duplicate.
- Around line 157-158: The dispatchReaction method call at line 157 is not
awaited, causing a race condition where the webhook returns 200 before async
reaction handlers complete. This is inconsistent with dispatchMessage and
dispatchAction which are both awaited. Make the dispatchReaction method async,
add await before the dispatchReaction call at line 157, and also await the
internal this.chat.processReaction call within the dispatchReaction method to
ensure all async operations complete before the webhook response is sent.
In `@packages/chat-adapter/src/message-mapper.ts`:
- Around line 77-98: The buildHistoryMessage method is setting conversationId to
an empty string in the raw message object, which loses the actual conversation
identity even though the threadId parameter is available and contains the real
conversation identifier. Replace the empty string value for conversationId with
the threadId parameter to preserve the conversation identity in the history raw
messages and maintain consistency with the thread state information.
In `@packages/chat-adapter/src/novu-context.ts`:
- Around line 18-33: The validation guard in getNovuContext() checks for the
existence of emitSignals and decodeThreadId functions but does not validate
emitResolve, which is called on line 32 in the resolve function. Add a
validation check for source.emitResolve to the existing guard clause that
validates adapter ownership, ensuring that all three methods (emitSignals,
decodeThreadId, and emitResolve) are present and are functions before proceeding
with the rest of the function logic.
In `@packages/chat-adapter/src/thread-id.ts`:
- Around line 24-36: The decodeThreadId function has incomplete validation that
allows invalid thread IDs to pass. The validation check currently verifies that
parts[2] and parts[3] are non-empty but does not validate parts[1] (platform) or
parts[4] (isDM flag). You need to extend the condition in the validation check
to also ensure that parts[1] is non-empty by adding it to the falsy checks
alongside parts[2] and parts[3]. Additionally, add validation for parts[4] to
ensure it is strictly '0' or '1' before setting the isDM property, rejecting any
other value as an invalid format to prevent silent defaulting to false for
malformed thread IDs.
In `@playground/nextjs/src/app/novu-agent/page.tsx`:
- Around line 7-11: The SimResult type definition is using the `interface`
keyword, but frontend code should use `type` instead. Change the `interface
SimResult` declaration to `type SimResult` to align with the frontend coding
guidelines where `type` is used for type definitions in client components and
`interface` is reserved for backend code.
---
Nitpick comments:
In `@packages/chat-adapter/src/thread-id.spec.ts`:
- Around line 28-32: The test for malformed ids in the it block labeled 'throws
on malformed ids' does not include test cases for empty platforms or invalid DM
flag values. Add the two missing test cases to the bad array that is iterated
over in the for loop: one case with an empty platform segment (novu::int:conv:0)
and one case with an invalid DM flag value (novu:slack:int:conv:X). This will
ensure that the decodeThreadId function properly validates these edge cases as
required by the validation logic.
In `@playground/nextjs/src/app/novu-agent/page.tsx`:
- Line 13: The NovuAgentPlayground component currently uses only a default
export, but the coding guidelines prefer named exports. Refactor the component
by first defining it as a named export function NovuAgentPlayground, then create
a separate default export statement that re-exports the named export. This
satisfies both the coding guideline requirement for named exports and the
Next.js App Router requirement for a default export on page components.
🪄 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: ab8d09b5-a66d-4d09-94e1-300e2129e8b5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
packages/chat-adapter/.gitignorepackages/chat-adapter/README.mdpackages/chat-adapter/package.jsonpackages/chat-adapter/project.jsonpackages/chat-adapter/src/adapter.integration.spec.tspackages/chat-adapter/src/adapter.tspackages/chat-adapter/src/index.tspackages/chat-adapter/src/message-mapper.tspackages/chat-adapter/src/novu-context.tspackages/chat-adapter/src/reply-client.spec.tspackages/chat-adapter/src/reply-client.tspackages/chat-adapter/src/signature.spec.tspackages/chat-adapter/src/signature.tspackages/chat-adapter/src/state-memory.tspackages/chat-adapter/src/thread-id.spec.tspackages/chat-adapter/src/thread-id.tspackages/chat-adapter/src/types.tspackages/chat-adapter/src/webhook-handler.tspackages/chat-adapter/tsconfig.jsonpackages/chat-adapter/vitest.config.tsplayground/nextjs/.env.exampleplayground/nextjs/package.jsonplayground/nextjs/src/app/api/novu-agent/agent.tsplayground/nextjs/src/app/api/novu-agent/route.tsplayground/nextjs/src/app/api/novu-agent/simulate/route.tsplayground/nextjs/src/app/novu-agent/page.tsx
| if (typeof source?.emitSignals !== 'function' || typeof source?.decodeThreadId !== 'function') { | ||
| throw new Error('getNovuContext() requires a thread owned by the Novu adapter'); | ||
| } | ||
|
|
||
| const threadId = thread.id; | ||
| const { platform } = source.decodeThreadId(threadId); | ||
|
|
||
| const emit = (signal: Signal) => source.emitSignals(threadId, [signal]); | ||
|
|
||
| return { | ||
| platform, | ||
| trigger: (workflowId, opts) => emit({ type: 'trigger', workflowId, to: opts?.to, payload: opts?.payload }), | ||
| setMetadata: (key, value) => emit({ type: 'metadata', action: 'set', key, value }), | ||
| deleteMetadata: (key) => emit({ type: 'metadata', action: 'delete', key }), | ||
| resolve: (summary) => source.emitResolve(threadId, summary), | ||
| }; |
There was a problem hiding this comment.
Guard emitResolve in adapter ownership validation.
Line 32 calls source.emitResolve(...), but Lines 18-19 only validate emitSignals and decodeThreadId. If an adapter passes those two checks but lacks emitResolve, resolve() fails at runtime.
Proposed fix
- if (typeof source?.emitSignals !== 'function' || typeof source?.decodeThreadId !== 'function') {
+ if (
+ typeof source?.emitSignals !== 'function' ||
+ typeof source?.decodeThreadId !== 'function' ||
+ typeof source?.emitResolve !== 'function'
+ ) {
throw new Error('getNovuContext() requires a thread owned by the Novu adapter');
}📝 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.
| if (typeof source?.emitSignals !== 'function' || typeof source?.decodeThreadId !== 'function') { | |
| throw new Error('getNovuContext() requires a thread owned by the Novu adapter'); | |
| } | |
| const threadId = thread.id; | |
| const { platform } = source.decodeThreadId(threadId); | |
| const emit = (signal: Signal) => source.emitSignals(threadId, [signal]); | |
| return { | |
| platform, | |
| trigger: (workflowId, opts) => emit({ type: 'trigger', workflowId, to: opts?.to, payload: opts?.payload }), | |
| setMetadata: (key, value) => emit({ type: 'metadata', action: 'set', key, value }), | |
| deleteMetadata: (key) => emit({ type: 'metadata', action: 'delete', key }), | |
| resolve: (summary) => source.emitResolve(threadId, summary), | |
| }; | |
| if ( | |
| typeof source?.emitSignals !== 'function' || | |
| typeof source?.decodeThreadId !== 'function' || | |
| typeof source?.emitResolve !== 'function' | |
| ) { | |
| throw new Error('getNovuContext() requires a thread owned by the Novu adapter'); | |
| } | |
| const threadId = thread.id; | |
| const { platform } = source.decodeThreadId(threadId); | |
| const emit = (signal: Signal) => source.emitSignals(threadId, [signal]); | |
| return { | |
| platform, | |
| trigger: (workflowId, opts) => emit({ type: 'trigger', workflowId, to: opts?.to, payload: opts?.payload }), | |
| setMetadata: (key, value) => emit({ type: 'metadata', action: 'set', key, value }), | |
| deleteMetadata: (key) => emit({ type: 'metadata', action: 'delete', key }), | |
| resolve: (summary) => source.emitResolve(threadId, summary), | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/chat-adapter/src/novu-context.ts` around lines 18 - 33, The
validation guard in getNovuContext() checks for the existence of emitSignals and
decodeThreadId functions but does not validate emitResolve, which is called on
line 32 in the resolve function. Add a validation check for source.emitResolve
to the existing guard clause that validates adapter ownership, ensuring that all
three methods (emitSignals, decodeThreadId, and emitResolve) are present and are
functions before proceeding with the rest of the function logic.
| export function decodeThreadId(threadId: string): NovuThreadId { | ||
| const parts = threadId.split(':'); | ||
| if (parts.length !== 5 || parts[0] !== PREFIX || !parts[2] || !parts[3]) { | ||
| throw new Error(`Invalid Novu thread id format: ${threadId}`); | ||
| } | ||
|
|
||
| return { | ||
| platform: decodeURIComponent(parts[1] ?? ''), | ||
| integrationIdentifier: decodeURIComponent(parts[2]), | ||
| conversationId: decodeURIComponent(parts[3]), | ||
| isDM: parts[4] === '1', | ||
| }; | ||
| } |
There was a problem hiding this comment.
Incomplete validation in decodeThreadId.
The validation on line 26 checks that parts[2] (integrationIdentifier) and parts[3] (conversationId) are non-empty, but does not validate parts[1] (platform). A thread id like 'novu::int:conv:0' would pass validation and return an empty platform, potentially causing downstream issues when constructing channel ids or routing messages.
Additionally, parts[4] (the DM flag) is not validated to be '0' or '1'—any other value silently defaults to false.
🛡️ Suggested validation improvement
export function decodeThreadId(threadId: string): NovuThreadId {
const parts = threadId.split(':');
- if (parts.length !== 5 || parts[0] !== PREFIX || !parts[2] || !parts[3]) {
+ if (
+ parts.length !== 5 ||
+ parts[0] !== PREFIX ||
+ !parts[1] ||
+ !parts[2] ||
+ !parts[3] ||
+ (parts[4] !== '0' && parts[4] !== '1')
+ ) {
throw new Error(`Invalid Novu thread id format: ${threadId}`);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/chat-adapter/src/thread-id.ts` around lines 24 - 36, The
decodeThreadId function has incomplete validation that allows invalid thread IDs
to pass. The validation check currently verifies that parts[2] and parts[3] are
non-empty but does not validate parts[1] (platform) or parts[4] (isDM flag). You
need to extend the condition in the validation check to also ensure that
parts[1] is non-empty by adding it to the falsy checks alongside parts[2] and
parts[3]. Additionally, add validation for parts[4] to ensure it is strictly '0'
or '1' before setting the isDM property, rejecting any other value as an invalid
format to prevent silent defaulting to false for malformed thread IDs.
| interface SimResult { | ||
| status: number; | ||
| routedTo: string; | ||
| replies: Array<{ url: string; payload: unknown }>; | ||
| } |
There was a problem hiding this comment.
Use type instead of interface for frontend type definitions.
Per coding guidelines, frontend code should use type for type definitions, while interface is reserved for backend code. This is a client component (frontend), so SimResult should be defined with type.
♻️ Proposed fix
-interface SimResult {
+type SimResult = {
status: number;
routedTo: string;
replies: Array<{ url: string; payload: unknown }>;
-}
+};📝 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.
| interface SimResult { | |
| status: number; | |
| routedTo: string; | |
| replies: Array<{ url: string; payload: unknown }>; | |
| } | |
| type SimResult = { | |
| status: number; | |
| routedTo: string; | |
| replies: Array<{ url: string; payload: unknown }>; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@playground/nextjs/src/app/novu-agent/page.tsx` around lines 7 - 11, The
SimResult type definition is using the `interface` keyword, but frontend code
should use `type` instead. Change the `interface SimResult` declaration to `type
SimResult` to align with the frontend coding guidelines where `type` is used for
type definitions in client components and `interface` is reserved for backend
code.
Source: Coding guidelines
| async appendToList(key: string, value: unknown, options?: { maxLength?: number; ttlMs?: number }): Promise<void> { | ||
| const list = this.lists.get(key) ?? []; | ||
| list.push(value); | ||
| if (options?.maxLength && list.length > options.maxLength) { | ||
| list.splice(0, list.length - options.maxLength); | ||
| } | ||
| this.lists.set(key, list); | ||
| } | ||
|
|
||
| async getList<T = unknown>(key: string): Promise<T[]> { | ||
| return [...((this.lists.get(key) as T[]) ?? [])]; | ||
| } |
There was a problem hiding this comment.
appendToList() accepts ttlMs, but the value is ignored and getList() never expires list keys. Chat SDK list-backed features such as history, transcripts, or adapter message-reference lists rely on this TTL being refreshed and enforced, so long-running processes can keep stale list data forever, return expired references, and grow memory beyond the configured retention.
Ran code and verified through T-Rex
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/chat-adapter/src/state-memory.ts
Line: 75-86
Comment:
**Honor list TTLs**
`appendToList()` accepts `ttlMs`, but the value is ignored and `getList()` never expires list keys. Chat SDK list-backed features such as history, transcripts, or adapter message-reference lists rely on this TTL being refreshed and enforced, so long-running processes can keep stale list data forever, return expired references, and grow memory beyond the configured retention.
How can I resolve this? If you propose a fix, please make it concise.…typing fixes NV-8063
Drop the bundled in-memory StateAdapter in favor of the official
@chat-adapter/state-memory package; consumers and the playground import
createMemoryState from it directly. Also widen NovuAdapter to the base Adapter
type so instances are assignable to new Chat({ adapters }).
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Stale comment
Risk: medium. Not approving: an unresolved medium-severity Security Agent finding remains on webhook signature verification (malformed hex can throw and return 500s instead of 401). ChmaraX and djabarovgeorge are already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Approver
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)
packages/chat-adapter/src/adapter.ts (1)
277-284:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGenerate unique fallback ids for empty Novu reply responses.
When
ReplyClient.sendreturns nomessageId, everypostMessagein the same conversation returnsnovu-reply:${conversationId}. Distinct outbound replies can then collide in any state/UI keyed byRawMessage.id.Proposed fix
private readonly webhookHandler: WebhookHandler; private readonly replyClient: ReplyClient; + private fallbackMessageCounter = 0; private chat: ChatInstance | null = null; @@ - const messageId = info?.messageId ?? `novu-reply:${decoded.conversationId}`; + const messageId = info?.messageId ?? this.nextFallbackMessageId(decoded.conversationId); @@ private outboundRaw(decoded: NovuThreadId, messageId: string): NovuRawMessage { @@ } + + private nextFallbackMessageId(conversationId: string): string { + this.fallbackMessageCounter += 1; + + return `novu-reply:${conversationId}:${Date.now()}:${this.fallbackMessageCounter}`; + } private emojiName(emoji: EmojiValue | string): string {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/chat-adapter/src/adapter.ts` around lines 277 - 284, The postMessage method generates the same fallback messageId for all messages in the same conversation when ReplyClient.send returns no messageId, causing collisions when messages are keyed by id. Replace the fallback id generation at the messageId assignment (where it currently uses novu-reply:${decoded.conversationId}) with a mechanism that generates a unique identifier for each call, such as by appending a timestamp, UUID, or counter to ensure each message gets a distinct id even when they share the same conversationId.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/chat-adapter/src/index.ts`:
- Line 52: Add a clarifying comment to the return statement in the
createNovuAdapter function explaining why the explicit `as NovuAdapter` cast is
necessary. The comment should document that NovuAdapterImpl implements
NovuTypedAdapter with specific generic type parameters (NovuThreadId and
NovuRawMessage), but the function return type requires the untyped
consumer-facing NovuAdapter interface, making the type widening cast required
and intentional. This prevents future developers from mistakenly removing the
cast thinking it is redundant.
---
Outside diff comments:
In `@packages/chat-adapter/src/adapter.ts`:
- Around line 277-284: The postMessage method generates the same fallback
messageId for all messages in the same conversation when ReplyClient.send
returns no messageId, causing collisions when messages are keyed by id. Replace
the fallback id generation at the messageId assignment (where it currently uses
novu-reply:${decoded.conversationId}) with a mechanism that generates a unique
identifier for each call, such as by appending a timestamp, UUID, or counter to
ensure each message gets a distinct id even when they share the same
conversationId.
🪄 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: d8418d44-6ca0-4bf4-a3d6-81a394608935
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
packages/chat-adapter/README.mdpackages/chat-adapter/package.jsonpackages/chat-adapter/src/adapter.integration.spec.tspackages/chat-adapter/src/adapter.tspackages/chat-adapter/src/index.tspackages/chat-adapter/src/novu-context.tspackages/chat-adapter/src/types.tsplayground/nextjs/package.jsonplayground/nextjs/src/app/api/novu-agent/agent.tsplayground/nextjs/src/app/api/novu-agent/simulate/route.ts
✅ Files skipped from review due to trivial changes (1)
- packages/chat-adapter/README.md
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/chat-adapter/package.json
- playground/nextjs/src/app/api/novu-agent/agent.ts
- packages/chat-adapter/src/novu-context.ts
- packages/chat-adapter/src/types.ts
- packages/chat-adapter/src/adapter.integration.spec.ts
- playground/nextjs/src/app/api/novu-agent/simulate/route.ts
…User fixes NV-8063 Surface the Novu subscriber to Chat SDK handlers two ways: the full rich profile (email, phone, avatar, locale, custom data) through the Novu-only escape hatch getNovuContext(thread).getSubscriber(), and the portable identity through the SDK-native adapter.getUser(userId) -> UserInfo. Align the inbound human author identity to the subscriber so message.author.userId === subscriberId (consistent across getParticipants and getUser); the platform-native author is preserved on message.raw.author.
There was a problem hiding this comment.
Stale comment
Risk: medium. Not approving: an unresolved medium-severity Security Agent finding remains on webhook signature verification (malformed hex can throw and return 500s instead of 401). ChmaraX and djabarovgeorge are already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Approver
There was a problem hiding this comment.
Stale comment
Risk: medium. Not approving: an unresolved medium-severity Security Agent finding remains on webhook signature verification (malformed hex can throw and return 500s instead of 401). ChmaraX and djabarovgeorge are already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Approver
…ges NV-8063 Commit delivery dedupe only after successful dispatch so transient failures can retry. Reject malformed HMAC hex without throwing, and honor fetchMessages limit/cursor pagination.
There was a problem hiding this comment.
Stale comment
Risk: medium. Not approving: an unresolved medium-severity Security Agent finding remains on webhook signature verification (malformed hex can throw and return 500s instead of 401). ChmaraX and djabarovgeorge are already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Approver
Run biome format on remaining chat-adapter files using single quotes. Add VS Code quoteStyle preferences so TypeScript imports match biome.
There was a problem hiding this comment.
Risk: medium. Not approving: an unresolved medium-severity Security Agent finding remains on webhook signature verification (malformed hex can throw and return 500s instead of 401). ChmaraX and djabarovgeorge are already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Approver
Bump version and update README with example app link and accurate usage docs.
…NV-8063 Add exports map, publish metadata, license, and keywords; tighten chat peer to ^4.30.0. Support env-var fallbacks in createNovuAdapter, and add @chat-adapter/shared dependency for upcoming shared error/util adoption.




Summary
Introduce
@novu/chat-sdk-adapter(packages/chat-adapter), a Chat SDK platform adapter that exposes all of Novu's normalized chat channels — Slack, WhatsApp, Microsoft Teams, Telegram, Email — as a single platform. Novu does the per-channel normalization (oneConversation+Subscriber+ history) and calls the developer's bridge; the developer's Chat SDK app becomes the brain. One handler set serves every channel with no per-channel code.What's included
@chat-adapter/novu, peer-depschat >=4.30.0(+ optionalreactfor JSX cards).novu-signature,t=,v1=scheme); mapsAgentBridgeRequestto Chat SDKThread/Message; server-truth routing (ongoing conversation ->onSubscribedMessage; brand-new ->onNewMention/onDirectMessageviaplatformContext.isDM);onResolveno-op; dedup viadeliveryId.postMessage/editMessage/addReactionserialize toreply/edit/addReactionsPOSTs authenticated with the Novu apiKey; edit-based streaming via the chat package's built-in cadence.apiBaseUrl+agentIdentifier); the inbound request'sreplyUrlis ignored, so a forged request can't exfiltrate the apiKey.StateAdapter(single-instance-safe); multi-instance bridges pass a shared state adapter.getNovuContext(thread)=>{ platform, trigger, setMetadata, deleteMetadata, resolve }. Ported Chat SDK bots ignore it and run unchanged.PUT /v1/agents/:id/bridge).playground/nextjs):/api/novu-agentlive bridge,/api/novu-agent/simulatecredential-free simulator, and/novu-agenttest UI.Out of scope (v1)
deleteMessage, modals, outbound-initiated DMs (openDM), code-driven channel provisioning, and Novu-side per-conversation turn serialization (tracked follow-up so zero-deps in-memory state is correct at horizontal scale).Follow-ups
@chat-adapter/novunpm name + chat-sdk.dev adapter listing (cross-team).Test plan
pnpm --filter @chat-adapter/novu test(unit: mappers, HMAC valid/invalid/replayed, reply-URL derivation)pnpm --filter @chat-adapter/novu build/novu-agenttest UI exercises onNewMention / onDirectMessage / onSubscribedMessage / onAction / onReaction with no credentials/api/novu-agent, send a real WhatsApp/Telegram message, confirm the echo returns on that channelCloses NV-8063
Made with Cursor
Summary by CodeRabbit
What changed
@novu/chat-sdk-adapterpackage to bridge Novu chat events into Chat SDK threads/messages.getNovuContext(thread)helper for Novu-specific capabilities.Greptile Summary
@novu/chat-sdk-adapterpackage that bridges Novu agent webhook events into Chat SDK threads, messages, and handlers.Confidence Score: 5/5
The changes appear merge-safe with no code issues identified.
The adapter and playground additions are covered by focused tests for the core bridge behaviors, signing, routing, reply handling, snapshots, and pagination.
What T-Rex did
Comments Outside Diff (7)
General comment
onMessagebridge requests withconversation.messageCount: 1, emptyhistory, andplatformContext.isDMset to bothfalseandtrue,handleWebhookreturned HTTP 200 but did not invokeonNewMentionoronDirectMessage. The PR claim says brand-new non-DM should route toonNewMentionand brand-new DM should route toonDirectMessage, but the after artifact's recorded handler calls omit both.packages/chat-adapter/src/adapter.tsroutes all message events throughthis.chat.processMessage(this, threadId, message, options)after only pre-subscribing ongoing conversations at lines 142-151. The adapter does not provide/preserve the platform channel context needed by the Chat SDK to classify a first message as a mention or direct message, so first-message server-truth cases are acknowledged but not dispatched to the expected new-conversation handlers.onMessage, pass enough channel/thread context into the Chat SDK (or explicitly choose the Chat SDK event path) somessageCount === 1 && history.length === 0 && !isDMinvokesonNewMention, and the same first-message case withisDM === trueinvokesonDirectMessage. Add an integration assertion for both first-message cases alongside the existing ongoing/action coverage.General comment
/api/novu-agent/simulateendpoint to let users exercise routing from the browser and display HTTP status, routedTo, and captured replies. In the realplayground/nextjsdev server, the page route loads, but POSTing both an ongoingonMessagescenario and a brand-new DM scenario to/api/novu-agent/simulatereturns HTTP 500. The response is Next.js' module-not-found error for@novu/chat-sdk-adapter, so the UI fetch would not receive the expected simulator JSON and cannot display the promised routed result.@novu/chat-sdk-adapter, but the actual dev-server compilation could not resolve that package from the playground app in this workspace install/state. The failing import is on the simulator/agent route path (playground/nextjs/src/app/api/novu-agent/simulate/route.tsimportscreateMemoryState/createNovuAdapter, andagent.tsalso imports the package).@novu/chat-sdk-adapteris resolvable by the Next.js playground in the repository-supported install/dev workflow. Verify the workspace package name/dependency is correct inplayground/nextjs/package.json, the lockfile/workspace links are updated, and Next.js can transpile/resolve the package from the app. Then re-run the real dev server and confirm/api/novu-agent/simulatereturns 200 JSON withstatus,routedTo, andrepliesfor ongoing and DM scenarios.General comment
onDirectMessageand new non-DM conversations should route toonNewMention. In the head runtime artifact, the synthetic webhook run delivered an ongoing message, a new DM (messageCount: 1, empty history,isDM: true, distinct conversation), a new non-DM mention (messageCount: 1, empty history,isDM: false, distinct conversation), action, reaction, and resolve. All six calls returnedHTTP 200, but the events captured from Chat SDK handlers only includesubscribed,action, andreaction; nodmormentionevent was delivered.NovuAdapterImpl.handleWebhookrelies on subscription pre-seeding atpackages/chat-adapter/src/adapter.ts:142-147and then delegates every onMessage tochat.processMessageatpackages/chat-adapter/src/adapter.ts:149-152/189-198. For brand-new conversations it leaves the thread unsubscribed, but the runtime path did not result in Chat SDK invoking the registered new-message handlers.chat.processMessagebefore dispatch. Add a regression test that assertsonDirectMessagefires formessageCount === 1 && history.length === 0 && isDMandonNewMentionfires for the corresponding non-DM case.General comment
/novu-agentplayground cannot successfully submit simulator requests. In the real Next.js app, POSTing to/api/novu-agent/simulatefails with HTTP 500 becausesrc/app/api/novu-agent/agent.tsimports@novu/chat-sdk-adapter, but that package cannot be resolved at runtime. As a result, the UI cannot display the requiredHTTP status,routedTo, and captured replies after interaction.playground/nextjs/src/app/api/novu-agent/agent.tsandsimulate/route.tsdepend on@novu/chat-sdk-adapter, but the workspace install/runtime resolution does not provide that package to the Next.js playground app under the current package graph.playground/nextjs: correct the package name/import if it was renamed, add the actual workspace package to the playground dependencies, and verifypnpm --dir playground/nextjs exec next devcan compile/api/novu-agent/simulate. Then retest the UI button flow for default and direct-message scenarios.General comment
packages/chat-adapter/package.jsondeclares the package as@novu/chat-sdk-adapter, while the validation contract and author's documented commands expect@chat-adapter/novu. Runningpnpm --filter @chat-adapter/novu testandpnpm --filter @chat-adapter/novu buildon head printsNo projects matched the filtersand exits 0 without executing tests or a build, so the advertised build/test contract is not actually validated by those commands. The playground is also wired to@novu/chat-sdk-adapter, not@chat-adapter/novu.packages/chat-adapter/package.jsonline 2 uses"name": "@novu/chat-sdk-adapter", andplayground/nextjs/package.jsonline 18 depends on"@novu/chat-sdk-adapter": "workspace:*", but the claimed workspace/package filter is@chat-adapter/novu.@chat-adapter/novupackage name, or update the published PR/test-plan contract and all setup guidance to use@novu/chat-sdk-adapter; ensure the documentedpnpm --filter ... testandbuildcommands match a real workspace project and execute scripts.General comment
/novu-agent, but running the real playground app on head without credentials renders the app-levelMissing NEXT_PUBLIC_CLERK_PUBLISHABLE_KEYscreen instead of the Novu agent playground. Because no simulator controls are visible, Playwright cannot triggeronNewMention,onDirectMessage,onSubscribedMessage,onAction, oronReactionthrough the real UI in the default credential-free setup.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEYbefore rendering route content. The changed/novu-agentpage and simulator may be credential-free with respect to Novu, but the playground route is not credential-free overall unless Clerk env is supplied./novu-agentrender independently of the Clerk-gated app shell, provide safe local defaults/stubs for Clerk in the playground, or update.env.example/startup contract so the route can be exercised without external credentials as claimed.General comment
POST /api/novu-agent/simulatewith body{"event":"onMessage","ongoing":false,"isDM":false,"text":"hello mention"}returnedHTTP/1.1 404 Not Foundand an HTML app not-found/missing-Clerk response in the real Next.js server. Other simulator paths in the same run returned structured200 OKJSON responses, so this is a contract mismatch for the claimed onNewMention event type.playground/nextjs/src/app/api/novu-agent/simulate/route.tslines 129-134, but the runtime request did not complete through that JSON handler path for the onNewMention representative request; it fell through to the app not-found/Clerk layout response.POST /api/novu-agent/simulateconsistently handles first-message/non-DM onMessage requests and returns the same structured JSON envelope as the other simulator event types. Add an endpoint test that posts the onNewMention body and asserts200 OK, JSON content type,routedTo:"onNewMention", and a captured reply.Reviews (4): Last reviewed commit: "chore(chat-adapter): align package metad..." | Re-trigger Greptile