feat(inbox): create connect chat channel connections#10671
feat(inbox): create connect chat channel connections#10671djabarovgeorge wants to merge 20 commits intonextfrom
Conversation
…controller and service - Implemented new endpoints for listing, creating, retrieving, and deleting channel connections and endpoints in the InboxController. - Updated InboxService to handle channel connection and endpoint operations. - Enhanced the API with new DTOs and commands for channel connections and endpoints. - Integrated feature flags for conditional access to channel features. - Updated module imports to include ChannelConnections and ChannelEndpoints modules.
…nctionality - Added support for Slack integration in the playground with new environment variables for Slack user ID and integration identifier. - Implemented a new DM endpoint creation feature in the Connect Chat page to resolve emails to Slack user IDs. - Updated the LinkUser component to manage channel endpoints more effectively using the Novu context. - Introduced new hooks for channel connections in the React package to streamline integration. - Updated pnpm-lock.yaml to include the internal SDK for Novu API.
- Deleted the `slack-connect.ts` API endpoint as it is no longer needed. - Updated the Slack user ID in the Connect Chat page to reflect the new default value.
- Removed console log statements for API response and data in the Connect Chat page to clean up the code and improve readability.
…hat page - Added a new input field for Slack user ID with a tooltip for user guidance. - Implemented logic to determine the effective Slack user ID for linking, prioritizing user input, environment variable, and a default value. - Updated the LinkUser component to use the dynamically determined Slack user ID.
❌ Deploy Preview for dashboard-v2-novu-staging failed. Why did it fail? →
|
📝 WalkthroughWalkthroughThis PR introduces comprehensive channel connection and OAuth linking features across the full stack: backend channel connection CRUD with Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as User Browser
participant Frontend as Frontend App
participant API as Novu API
participant OAuth as Slack OAuth
participant Callback as OAuth Callback
rect rgba(100, 150, 255, 0.5)
Note over Browser,Callback: Slack OAuth Connect Flow (mode: 'connect' or 'link_user')
Browser->>Frontend: Click "Connect Slack"
Frontend->>API: POST /inbox/chat/oauth<br/>(mode, connectionMode, scope)
API->>API: generateChatOAuthUrl.execute()
API-->>Frontend: { url: "https://slack.com/oauth..." }
Frontend->>Browser: window.open(url, '_blank')
Browser->>OAuth: Request OAuth Scopes
OAuth->>OAuth: User Authenticates
OAuth->>Callback: Redirect to callback URL<br/>(code, state)
Callback->>API: POST /slack/oauth/callback<br/>(code, state)
API->>API: Validate state & decrypt<br/>(contains mode, connectionMode, subscriberId)
alt mode === 'connect'
API->>API: Exchange code for workspace tokens
API->>API: Create CHANNEL_CONNECTION<br/>(with connectionMode)
alt connectionMode === 'shared'
API->>API: omit subscriberId
else connectionMode === 'subscriber'
API->>API: include subscriberId
end
else mode === 'link_user'
API->>API: Exchange code for user token
API->>API: Create SLACK_USER endpoint<br/>(subscriber-scoped)
end
API->>Browser: Redirect with success message
Frontend->>API: Poll /inbox/channel-connections<br/>(to detect new connection)
API-->>Frontend: { data: ChannelConnection[] }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
|
Hey there and thank you for opening this pull request! 👋 We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted. Your PR title is: Requirements:
Expected format: Details: |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1900527. Configure here.
| cleanupCreateResolved(); | ||
| cleanupDeleteResolved(); | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Solid.js onMount return value ignored, event listeners leak
High Severity
The LinkUser component returns a cleanup function from onMount, but Solid.js 1.x ignores the onMount return value (unlike React's useEffect). The cleanupCreateResolved and cleanupDeleteResolved listener unsubscribe functions are never called, so the event listeners persist after the component unmounts. The correct pattern — used by useChannelConnection.ts and useChannelEndpoint.ts in this same PR — is to call onCleanup(…) inside the onMount callback.
Reviewed by Cursor Bugbot for commit 1900527. Configure here.
| setEndpoint(data as ChannelEndpointResponse); | ||
| propsRef.current.onSuccess?.(data as ChannelEndpointResponse); | ||
| } | ||
| setIsFetching(false); |
There was a problem hiding this comment.
Unfiltered global events cause cross-component state corruption
Medium Severity
The channel-endpoint.create.resolved and channel-endpoint.get.resolved event handlers don't filter by the hook's own identifier. Any endpoint creation or get resolution — even for a completely different endpoint — overwrites this hook's state. The existing useSubscription hook properly filters by topicKey and identifier before mutating state. The same unfiltered pattern exists in useChannelConnection for channel-connection.get.resolved.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 1900527. Configure here.
| </For> | ||
| </Show> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Duplicated renderer component could be shared function
Low Severity
ChannelComponentsRenderer is an exact character-for-character duplicate of SubscriptionComponentsRenderer (lines 132–154). Both accept the same props and render the same Show/For/Portal/Root tree. A single shared component parameterized by its element list and nodes map would eliminate the duplication.
Reviewed by Cursor Bugbot for commit 1900527. Configure here.
…rops - Replaced slackUserId prop with type and endpoint props in LinkUser and DefaultLinkUser components for improved flexibility. - Enhanced endpoint matching logic to avoid duplicates based on type and endpoint combination. - Updated related documentation and examples to reflect changes in prop usage.
…to support new parameters - Added optional parameters `channel`, `providerId`, and `contextKeys` to `ListChannelConnectionsArgs` type for enhanced filtering capabilities. - Updated `InboxService` methods to incorporate the new parameters in search queries, allowing for more granular data retrieval. - Modified `LinkUser` component to accept and utilize `contextKeys` for improved endpoint deduplication logic.
@novu/js
@novu/nextjs
novu
@novu/providers
@novu/react
@novu/react-native
@novu/shared
commit: |
- Introduced optional parameters `endpointType` and `endpointData` across various controllers and DTOs to facilitate automatic channel endpoint creation during the OAuth process. - Updated the `GenerateChatOauthUrl` and related use cases to handle new parameters, streamlining the integration process for Slack and Microsoft Teams. - Enhanced the `connectAndLink` method in the ChannelConnections module to support the new functionality, allowing for a single API call to create both channel connections and endpoints. - Modified relevant components and hooks in the React and Next.js applications to utilize the new parameters, improving user experience and reducing the need for additional API calls.
- Introduced the SlackConnectButton component to the UI library, enhancing integration with Slack. - Updated appearance keys and types to include SlackConnectButton properties for better customization. - Modified relevant components and pages to utilize the new SlackConnectButton, improving user experience in the Connect Chat flow. - Removed playground directory from cursorignore to streamline development.
- Eliminated the optional parameters `endpointType` and `endpointData` from various controllers, DTOs, and use cases to simplify the chat OAuth process. - Updated the `GenerateChatOauthUrl` and related methods to remove references to the removed parameters, streamlining the integration for Slack and Microsoft Teams. - Adjusted the `connectAndLink` method in the ChannelConnections module to reflect these changes, ensuring a cleaner API interaction. - Modified relevant components and hooks in the React and Next.js applications to remove dependencies on the removed parameters, enhancing code clarity and maintainability.
- Added `userScope` and `mode` parameters to the chat OAuth flow in the Inbox and Integrations controllers, allowing for more granular control over OAuth requests. - Updated the `GenerateChatOauthUrl` and related DTOs to include the new parameters, facilitating user-level OAuth scope requests. - Implemented logic in the Slack OAuth callback to handle the new `link_user` mode, enabling linking of subscribers to Slack users. - Modified relevant components and hooks in the React and Next.js applications to support the new parameters, improving the user experience during the OAuth process.
…andling - Changed the `subscriberId` prop in `LinkSlackUserProps` to be optional, allowing for more flexible usage. - Updated the `LinkSlackUser` component to use a fallback for `subscriberId` when not provided. - Removed `subscriberId` from `DefaultLinkSlackUserProps` to align with the updated prop definition. - Introduced a context variable in the `ConnectChatPage` to enhance the linking process with contextual information.
- Added the SlackLinkUser component to facilitate linking subscribers via Slack OAuth. - Updated type definitions to replace LinkSlackUser with SlackLinkUser for consistency across the codebase. - Modified relevant imports and component references in the UI library and examples to reflect the new naming convention. - Enhanced the ConnectChatPage to utilize the new SlackLinkUser component, improving the user experience during the linking process.
…ntifiers - Introduced `slack-constants.ts` to define default values for connection and integration identifiers. - Updated `SlackConnectButton` and `SlackLinkUser` components to utilize these constants, improving code maintainability and consistency. - Refactored relevant logic to use the new constants for better clarity in connection handling.
- Added `connectionMode` property to `CreateChannelConnectionRequestDto`, `CreateChannelConnectionCommand`, and related DTOs to support different connection strategies. - Implemented validation logic in the `CreateChannelConnection` use case to enforce rules based on the selected `connectionMode` (either 'subscriber' or 'shared'). - Updated relevant components and hooks across the UI to handle the new `connectionMode` parameter, enhancing flexibility in connection management. - Modified the `SlackConnectButton` and `ConnectChat` components to incorporate `connectionMode`, improving user experience during the connection process.
…d update related types - Renamed `SlackConnectButton` to `ChannelConnectButton` across components and types for consistency. - Updated appearance keys and types to reflect the new naming convention, enhancing clarity in the UI. - Modified the `ConnectChatPage` to utilize the updated `ChannelConnectButton`, improving the integration experience. - Adjusted related imports and context handling to support the new structure, ensuring seamless functionality.
…ser feedback - Changed button text in LinkUser component to 'Connected' and 'Connect your Slack account' for clarity. - Enhanced SlackLinkUser component with visual feedback using icons to indicate connection status. - Updated appearance keys to include 'linkSlackUserButtonIcon' for better styling control.
- Added new types for SlackLinkUser appearance callbacks to improve customization options. - Updated SlackLinkUser component to utilize the new appearance callbacks for dynamic styling based on user link status. - Modified ConnectChatPage to demonstrate the new appearance capabilities, enhancing user feedback during the linking process.
…ditional parameters - Modified the `get` method calls in `InboxService` to include an `undefined` parameter and a `false` flag, enhancing the flexibility of the HTTP requests. - Updated `ConnectChatPage` to comment out the Slack button labels, preparing for potential future customization or changes in the UI.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| import { novuConfig } from '@/utils/config'; | ||
|
|
||
| const INTEGRATION_IDENTIFIER = process.env.NEXT_PUBLIC_NOVU_SLACK_INTEGRATION_IDENTIFIER ?? 'slack'; | ||
| const CONNECTION_IDENTIFIER = 'slack-workspace-connection'; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/api/src/app/channel-connections/usecases/create-channel-connection/create-channel-connection.usecase.ts (1)
55-77:⚠️ Potential issue | 🟠 MajorReject unsupported
connectionModevalues explicitly.Right now, an invalid
connectionModecan still pass validation through the fallback path (Lines 75-77) whensubscriberIdorcontextis provided. This weakens the mode contract for a security-sensitive flow.Suggested fix
private validateResourceOrContext(command: CreateChannelConnectionCommand) { const { subscriberId, context, connectionMode } = command; + if (connectionMode && connectionMode !== 'shared' && connectionMode !== 'subscriber') { + throw new BadRequestException('connectionMode must be either "shared" or "subscriber"'); + } + if (connectionMode === 'shared') { if (!context) { throw new BadRequestException('context is required when connectionMode is "shared"'); }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/channel-connections/usecases/create-channel-connection/create-channel-connection.usecase.ts` around lines 55 - 77, The code currently allows any connectionMode value to slip through if subscriberId or context are present; add an explicit validation of allowed modes (e.g., only 'shared' and 'subscriber') at the start of the validation block and throw BadRequestException for invalid values. Update the logic around the connectionMode variable in create-channel-connection.usecase (the same scope that already throws BadRequestException for 'shared' and 'subscriber') to check an allowedModes set and immediately reject unknown modes before falling back to the subscriberId/context checks.apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-chat-oauth-url.command.ts (1)
12-18:⚠️ Potential issue | 🟠 MajorReject incomplete
link_userpayloads before generating the OAuth URL.
link_useronly works when both the subscriber and the target connection are known, but this command still accepts that mode with neithersubscriberIdnorconnectionIdentifier. That pushes an impossible state into the signed OAuth payload and only surfaces after the user has already completed the Slack consent flow.As per coding guidelines,
apps/api/**: Check for proper error handling and input validation.Also applies to: 34-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-chat-oauth-url.command.ts` around lines 12 - 18, The DTO in generate-chat-oauth-url.command.ts currently allows creating a "link_user" OAuth payload without both identifiers, which leads to invalid signed payloads; update the validation in the GenerateChatOAuthUrlCommand handler (or the DTO validation logic) to explicitly reject requests intended for link_user mode unless both subscriberId and connectionIdentifier are present, returning a clear client error (e.g., BadRequest) when either is missing; ensure the check references the subscriberId and connectionIdentifier fields and is executed before any OAuth payload/signing or URL generation.apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.ts (1)
196-203:⚠️ Potential issue | 🔴 CriticalRestore the environment-derived OAuth callback URL.
Line 202 hardcodes a public ngrok host, so Slack will send authorization codes/state to that tunnel instead of the current API deployment. That breaks every non-local environment and turns a dev-only tunnel into part of the auth boundary.
As per coding guidelines, `apps/api/**`: Review with focus on security, authentication, and authorization.🐛 Suggested fix
static buildRedirectUri(): string { if (!process.env.API_ROOT_URL) { throw new Error('API_ROOT_URL environment variable is required'); } const baseUrl = process.env.API_ROOT_URL.replace(/\/$/, ''); // Remove trailing slash - return `https://536c-84-110-187-162.ngrok-free.app${CHAT_OAUTH_CALLBACK_PATH}`; - // return `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}`; + return `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.ts` around lines 196 - 203, The buildRedirectUri method currently returns a hardcoded ngrok URL; replace that with a URL constructed from the environment-derived base by restoring the commented return so it uses process.env.API_ROOT_URL (after removing any trailing slash) concatenated with CHAT_OAUTH_CALLBACK_PATH; keep the existing check that throws when API_ROOT_URL is missing and ensure the function returns `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}` rather than the static ngrok host.
🟡 Minor comments (15)
packages/js/src/ui/icons/CheckCircleFill.tsx-1-4 (1)
1-4:⚠️ Potential issue | 🟡 MinorRename file to kebab-case and add a blank line before
return.Two guideline violations here: filename casing at Line 1 (
CheckCircleFill.tsx) and missing blank line beforereturnat Line 4.Suggested fix
-// packages/js/src/ui/icons/CheckCircleFill.tsx +// packages/js/src/ui/icons/check-circle-fill.tsx export const CheckCircleFill = (props?: JSX.HTMLAttributes<SVGSVGElement>) => { + return ( <svg viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg" {...props}>As per coding guidelines: "
**/*: File/directory names should use lowercase with dashes" and "**/*.{ts,tsx,js,jsx}: Include a blank line before everyreturnstatement."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/icons/CheckCircleFill.tsx` around lines 1 - 4, Rename the file from CheckCircleFill.tsx to kebab-case (e.g., check-circle-fill.tsx) and update any imports; inside the CheckCircleFill component (export const CheckCircleFill) add a blank line immediately before the return statement so there is an empty line separating the function body and the return.packages/js/src/ui/icons/SlackColored.tsx-1-4 (1)
1-4:⚠️ Potential issue | 🟡 MinorApply naming and return-spacing guidelines in this new icon file.
Please rename
SlackColored.tsxto kebab-case and insert a blank line beforereturnat Line 4.Suggested fix
-// packages/js/src/ui/icons/SlackColored.tsx +// packages/js/src/ui/icons/slack-colored.tsx export const SlackColored = (props?: JSX.HTMLAttributes<SVGSVGElement>) => { + return ( <svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg" {...props}>As per coding guidelines: "
**/*: File/directory names should use lowercase with dashes" and "**/*.{ts,tsx,js,jsx}: Include a blank line before everyreturnstatement."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/icons/SlackColored.tsx` around lines 1 - 4, The file and component violate naming and spacing rules: rename the file SlackColored.tsx to kebab-case (e.g., slack-colored.tsx) and inside the SlackColored component (export const SlackColored) insert a single blank line immediately before the return statement so there is an empty line between the function opening and the return.packages/react/src/components/connect-chat/DefaultConnectChat.tsx-36-38 (1)
36-38:⚠️ Potential issue | 🟡 MinorAdd a blank line before the callback
returnfor guideline compliance.Suggested patch
const mount = useCallback( (element: HTMLElement) => { + return novuUI.mountComponent({ name: 'ConnectChat',As per coding guidelines
**/*.{ts,tsx,js,jsx}: Include a blank line before everyreturnstatement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/connect-chat/DefaultConnectChat.tsx` around lines 36 - 38, In DefaultConnectChat.tsx inside the anonymous callback "(element: HTMLElement) => { ... }" add a blank line immediately before the "return novuUI.mountComponent({ name: 'ConnectChat', ... })" statement to satisfy the style rule requiring a blank line before every return; update only that callback body so the return is preceded by a single empty line.packages/react/src/components/connect-chat/ConnectChat.tsx-13-15 (1)
13-15:⚠️ Potential issue | 🟡 MinorPlease insert blank lines before both
returnstatements.Suggested patch
const options: NovuUIOptions = useMemo(() => { + return { container, appearance, @@ export const ConnectChat = React.memo((props: ConnectChatProps) => { + return <ConnectChatInternal {...props} />; });As per coding guidelines
**/*.{ts,tsx,js,jsx}: Include a blank line before everyreturnstatement.Also applies to: 30-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/connect-chat/ConnectChat.tsx` around lines 13 - 15, Insert a blank line immediately before each return statement in ConnectChat.tsx to satisfy the coding guideline: add a blank line before the return inside the useMemo callback that builds `options: NovuUIOptions` (the `useMemo(() => { return { container, ... } })` block) and also add a blank line before the other return at lines ~30-31 (the component's `return` that renders JSX). Ensure both return lines have a preceding empty line.packages/react/src/components/slack-link-user/DefaultSlackLinkUser.tsx-34-36 (1)
34-36:⚠️ Potential issue | 🟡 MinorAdd a blank line before the callback
return.Suggested patch
const mount = useCallback( (element: HTMLElement) => { + return novuUI.mountComponent({ name: 'SlackLinkUser',As per coding guidelines
**/*.{ts,tsx,js,jsx}: Include a blank line before everyreturnstatement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/slack-link-user/DefaultSlackLinkUser.tsx` around lines 34 - 36, In DefaultSlackLinkUser.tsx, inside the callback `(element: HTMLElement) => { ... }` that currently returns `novuUI.mountComponent({ name: 'SlackLinkUser', ... })`, insert a blank line immediately before the `return` statement to comply with the repository style rule requiring a blank line before every return; update the anonymous callback body around the `return` in that function accordingly.packages/react/src/components/slack-connect-button/DefaultSlackConnectButton.tsx-40-42 (1)
40-42:⚠️ Potential issue | 🟡 MinorInsert a blank line before the callback
return.Suggested patch
const mount = useCallback( (element: HTMLElement) => { + return novuUI.mountComponent({ name: 'SlackConnectButton',As per coding guidelines
**/*.{ts,tsx,js,jsx}: Include a blank line before everyreturnstatement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/slack-connect-button/DefaultSlackConnectButton.tsx` around lines 40 - 42, In DefaultSlackConnectButton update the anonymous callback (the arrow function with signature "(element: HTMLElement) => { ... }") so there is a blank line immediately before the "return" statement; locate the callback that calls novuUI.mountComponent and insert a single empty line above the return to satisfy the project rule requiring a blank line before every return.packages/react/src/components/slack-connect-button/SlackConnectButton.tsx-13-15 (1)
13-15:⚠️ Potential issue | 🟡 MinorPlease add blank lines before both
returnstatements.Suggested patch
const options: NovuUIOptions = useMemo(() => { + return { container, appearance, @@ export const SlackConnectButton = React.memo((props: SlackConnectButtonProps) => { + return <SlackConnectButtonInternal {...props} />; });As per coding guidelines
**/*.{ts,tsx,js,jsx}: Include a blank line before everyreturnstatement.Also applies to: 30-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/slack-connect-button/SlackConnectButton.tsx` around lines 13 - 15, Add a blank line immediately before each return statement in SlackConnectButton.tsx: insert one blank line before the return inside the useMemo callback that builds the options (the NovuUIOptions = useMemo(...) callback) and also insert a blank line before the component's JSX return (the return in the SlackConnectButton component itself) so both returns have a preceding empty line per the coding guideline.packages/react/src/hooks/useChannelConnections.ts-47-80 (1)
47-80:⚠️ Potential issue | 🟡 MinorDuplicate callback invocations in
fetchConnectionsand event listener.The
fetchConnectionsfunction (lines 47–55) processes the API response directly and invokesonSuccess/onErrorcallbacks. Thechannel-connections.listhelper in the SDK simultaneously emits thechannel-connections.list.resolvedevent after returning its response, which theuseEffecthook listens to (lines 68–80) and processes identically. This results in both code paths executing for every list call, causing callbacks to be invoked twice.Consider either:
- Removing callback invocations from
fetchConnectionsand relying solely on the event listener, or- Removing the resolved event listener and relying on direct response handling in
fetchConnections.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/hooks/useChannelConnections.ts` around lines 47 - 80, The fetchConnections function and the 'channel-connections.list.resolved' event handler both process the same response and call onSuccess/onError, causing duplicate callback invocations; pick one approach—either remove the onSuccess/onError handling from fetchConnections (leave fetchConnections to only call novu.channelConnections.list and update local loading state, letting the novu.on('channel-connections.list.resolved') handler perform setConnections/setError and invoke propsRef.current.onSuccess/onError), or remove the resolved event listener in the useEffect and keep the direct response handling inside fetchConnections (remove the novu.on('channel-connections.list.resolved' registration and its cleanup variables). Ensure you reference and update propsRef.current, setConnections, setError, setIsFetching, and setIsLoading consistently depending on which path you choose so callbacks run exactly once.packages/react/src/hooks/useChannelConnections.ts-63-93 (1)
63-93:⚠️ Potential issue | 🟡 MinorMissing
isLoadingreset in resolved event handler.The
channel-connections.list.resolvedhandler (lines 68-80) setsisFetching(false)but does not resetisLoading. When the hook is driven purely by external events (not viafetchConnections),isLoadingmay remain stale if it was previously set totrue.Proposed fix
const cleanupListResolved = novu.on('channel-connections.list.resolved', ({ data, error: resolvedError }) => { const { onSuccess, onError } = propsRef.current; if (resolvedError) { setError(resolvedError as NovuError); onError?.(resolvedError as NovuError); } else if (data) { setConnections(data as ChannelConnectionResponse[]); onSuccess?.(data as ChannelConnectionResponse[]); } + setIsLoading(false); setIsFetching(false); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/hooks/useChannelConnections.ts` around lines 63 - 93, The 'channel-connections.list.resolved' event handler in useChannelConnections should also reset the isLoading state; update the handler (the callback registered with novu.on('channel-connections.list.resolved')) to call setIsLoading(false) when the response is resolved (both on error and success) — either by adding setIsLoading(false) inside both branches where you currently call setError/setConnections or by calling it once after handling resolvedError/data; reference the propsRef usage and setIsFetching/setError/setConnections to locate the exact callback to modify.packages/js/src/ui/api/hooks/useChannelEndpoint.ts-61-64 (1)
61-64:⚠️ Potential issue | 🟡 MinorEvent handler may update cache with unrelated endpoint data.
The
channel-endpoint.create.resolvedhandler mutates the local cache unconditionally. If another component creates an endpoint for a different subscriber/integration, this hook will incorrectly cache that endpoint. Consider filtering byoptions.integrationIdentifier,options.connectionIdentifier, andoptions.subscriberIdbefore callingmutate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/api/hooks/useChannelEndpoint.ts` around lines 61 - 64, The handler registered on currentNovu for 'channel-endpoint.create.resolved' currently calls mutate unconditionally; modify it to first inspect the event payload (the { data } object) and compare its integrationIdentifier, connectionIdentifier, and subscriberId against the hook's options (options.integrationIdentifier, options.connectionIdentifier, options.subscriberId), and only call mutate((data as ChannelEndpointResponse) ?? null) and setLoading(false) when the identifiers match the current options; leave the handler a no-op if they don't match so unrelated endpoint creations don't overwrite the cache.packages/js/src/ui/components/connect-chat/ConnectChat.tsx-69-74 (1)
69-74:⚠️ Potential issue | 🟡 Minor
onConnectSuccessis not called whenconnectionIdentifieris undefined.When
connectionIdentifieris not provided, the OAuth URL opens butonConnectSuccessnever fires. This could leave consumers without a way to know the OAuth flow was initiated. Consider either:
- Always calling
onConnectSuccess(perhaps with the result data), or- Documenting that
connectionIdentifieris required to receive the success callback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/connect-chat/ConnectChat.tsx` around lines 69 - 74, The current logic in ConnectChat.tsx only invokes props.onConnectSuccess when props.connectionIdentifier is truthy, so consumers without a connectionIdentifier never get a success callback; change the flow so props.onConnectSuccess is always called after opening the OAuth URL (e.g., pass the result.data or an object containing connectionIdentifier: props.connectionIdentifier ?? null and the URL), ensuring the callback receives useful context; update the block that checks result.data?.url (and uses window.open) to always call props.onConnectSuccess with the appropriate payload instead of gating it on props.connectionIdentifier.playground/nextjs/src/lib/slack-dm-endpoint-connect.ts-128-148 (1)
128-148:⚠️ Potential issue | 🟡 MinorPagination limit may miss existing endpoints or connections.
Both
endpoints.listandconnections.listuselimit: 100without handling pagination. If a subscriber has more than 100 endpoints or connections, the check for existingslack_userendpoint or connection lookup may produce incorrect results.Consider either:
- Increasing the limit if 100 is known to be sufficient for this use case
- Implementing pagination to fetch all results
- Adding a comment documenting this limitation
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/nextjs/src/lib/slack-dm-endpoint-connect.ts` around lines 128 - 148, The current checks use novu.channelEndpoints.list and novu.channelConnections.list with limit: 100 which can miss results; update the logic to fully paginate both lists before checking for existing Slack entries (rather than assuming 100 is sufficient): implement a loop (or use the SDK's pagination helper) to fetch all pages for endpoints and connections, accumulate results, then perform the existing alreadyLinked check (ep.type === SLACK_USER_TYPE && endpointData.userId === slackUserId) and the connection lookup against the complete datasets (or, if you intentionally bound to 100, add a clear comment noting the limitation and reason).packages/js/src/ui/api/hooks/useChannelEndpoint.ts-32-41 (1)
32-41:⚠️ Potential issue | 🟡 MinorMissing error handling leaves
loadingstate stuck if API throws.If
novuAccessor().channelEndpoints.create(args)throws an exception,setLoading(false)on line 38 is never reached, leaving the hook in a permanent loading state.Proposed fix
const create = async (args: CreateChannelEndpointArgs) => { setLoading(true); - const response = await novuAccessor().channelEndpoints.create(args); - if (response.data) { - mutate(response.data); + try { + const response = await novuAccessor().channelEndpoints.create(args); + if (response.data) { + mutate(response.data); + } + + return response; + } finally { + setLoading(false); } - setLoading(false); - - return response; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/api/hooks/useChannelEndpoint.ts` around lines 32 - 41, The create function can throw and leave loading stuck; wrap the API call in a try/catch/finally inside create (the function that calls novuAccessor().channelEndpoints.create and calls setLoading(true)) so setLoading(false) runs in finally; in the try block call the API and mutate(response.data) as currently done, in catch rethrow or return the error as appropriate, and ensure any error is either logged or propagated after finally so callers can handle it.apps/api/src/app/inbox/inbox.controller.ts-815-821 (1)
815-821:⚠️ Potential issue | 🟡 MinorSame non-null assertion issue as in
listChannelConnections.Lines 819-820 have the same non-null assertions on
totalCountandtotalCountCapped. Apply the same fix pattern suggested above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/inbox/inbox.controller.ts` around lines 815 - 821, The return object uses non-null assertions on result.totalCount and result.totalCountCapped; replace those unsafe "!" assertions with a safe default (e.g., use the nullish coalescing operator) so that totalCount: result.totalCount ?? 0 and totalCountCapped: result.totalCountCapped ?? 0 (or another acceptable fallback) are returned; update the return in the block that maps result.data with mapChannelEndpointEntityToDto to avoid possible undefined values.apps/api/src/app/inbox/inbox.controller.ts-700-706 (1)
700-706:⚠️ Potential issue | 🟡 MinorNon-null assertions on optional fields may cause runtime errors.
Lines 704-705 use non-null assertions (
!) ontotalCountandtotalCountCapped. If the underlying use case doesn't guarantee these values are present, this could lead to unexpectedundefinedvalues being returned to clients.Consider providing default values or making these fields optional in the response DTO.
Proposed fix with default values
return { data: result.data.map(mapChannelConnectionEntityToDto), next: result.next, previous: result.previous, - totalCount: result.totalCount!, - totalCountCapped: result.totalCountCapped!, + totalCount: result.totalCount ?? 0, + totalCountCapped: result.totalCountCapped ?? false, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/inbox/inbox.controller.ts` around lines 700 - 706, The return uses non-null assertions on result.totalCount and result.totalCountCapped which can throw if undefined; update the return in the controller that builds the response (the object using mapChannelConnectionEntityToDto) to either provide safe defaults (e.g., use the nullish coalescing operator: totalCount: result.totalCount ?? 0, totalCountCapped: result.totalCountCapped ?? false) or change the response DTO to mark those properties optional and pass them through without `!`; update the Inbox controller return building code and corresponding DTO/interface (if you choose the optional route) so types align and no `!` is used.
🧹 Nitpick comments (17)
packages/js/scripts/size-limit.mjs (1)
18-18: Document this size-budget increase to avoid silently loosening SDK guardrails.Bumping the UMD cap is reasonable for this feature, but please add an inline rationale (what added bytes, expected follow-up to reduce) so future increases remain intentional and auditable.
Suggested small update
{ name: 'UMD minified', filePath: umdPath, - limitInBytes: 213_000, + // Increased for Connect Chat flow additions in PR `#10671`; keep this budget tight and revisit on next bundle optimization pass. + limitInBytes: 213_000, },As per coding guidelines,
packages/js/**changes should be reviewed with focus on bundle size, and this line directly adjusts the bundle size gate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/scripts/size-limit.mjs` at line 18, Add an inline rationale comment next to the limitInBytes setting in packages/js/scripts/size-limit.mjs explaining why the UMD cap was increased (e.g., which feature or files added the ~X bytes), reference any PR or issue that introduced the change, and note the expected follow-up to reduce size (e.g., tree-shake refactor, split bundle, or defer assets). Locate the unique symbol limitInBytes and insert a concise one- or two-sentence comment immediately adjacent to it that documents the delta and planned remediation so future reviewers can audit the budget change.apps/api/src/app/channel-connections/dtos/create-channel-connection-request.dto.ts (1)
39-45: Avoid duplicating allowed mode literals across layers.
'subscriber' | 'shared'is now repeated in validators/Swagger across DTO + command. Consider a shared runtime constant so@IsIn(...)and Swagger enum stay in sync with the exported type.As per coding guidelines: "`{apps/api,libs/application-generic}/**/*.dto.ts`: Align `@ApiProperty` / `@ApiPropertyOptional` decorators with TypeScript optional properties (`?`) in DTO files."♻️ Suggested direction
// packages/shared/src/types/channel-connection.ts +export const CONNECTION_MODES = ['subscriber', 'shared'] as const; +export type ConnectionMode = (typeof CONNECTION_MODES)[number]; // apps/api/...create-channel-connection-request.dto.ts -import { ConnectionMode, ContextPayload } from '@novu/shared'; +import { CONNECTION_MODES, ConnectionMode, ContextPayload } from '@novu/shared'; ... - enum: ['subscriber', 'shared'], + enum: CONNECTION_MODES, ... - `@IsIn`(['subscriber', 'shared']) + `@IsIn`(CONNECTION_MODES) connectionMode?: ConnectionMode;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/channel-connections/dtos/create-channel-connection-request.dto.ts` around lines 39 - 45, The DTO duplicates the allowed connection mode literals for Swagger and validation (the connectionMode property uses enum: ['subscriber','shared'] and `@IsIn`(['subscriber','shared'])); replace these literal arrays with a single shared runtime constant or exported enum (e.g., reuse the exported ConnectionMode type/enum or a CONNECTION_MODES array) and import it into create-channel-connection-request.dto.ts, then reference that constant in both the `@ApiProperty` enum and the `@IsIn`(...) decorator for the connectionMode property; also confirm the decorator used (`@ApiPropertyOptional`) matches the TypeScript optional signature of connectionMode?.packages/nextjs/src/app-router/index.ts (1)
13-17: Re-exports are fine; remember semver-minor for@novu/nextjs.These additions expand the public package API and should be reflected in release/version classification.
As per coding guidelines,
packages/**/*: New exports are minor-version API changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nextjs/src/app-router/index.ts` around lines 13 - 17, You added new public exports (SlackConnectButton, SlackLinkUser, SubscriptionButton, SubscriptionPreferences, useNovu) in packages/nextjs/src/app-router/index.ts which constitutes a semver-minor API change; update the package release metadata to mark `@novu/nextjs` as a minor release (not patch) and ensure any changelog/release tooling, package.json release-type, or release notes reflect these new public exports so the versioning follows the guidelines for packages/**/* new exports.packages/js/src/ui/icons/index.ts (1)
9-24: Use lowercase-dash module names for new icon files.The new paths use PascalCase. Please align new icon filenames/exports with the repo naming rule.
Suggested export update (after file rename)
-export * from './CheckCircleFill'; +export * from './check-circle-fill'; ... -export * from './SlackColored'; +export * from './slack-colored';As per coding guidelines,
**/*: File/directory names should use lowercase with dashes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/icons/index.ts` around lines 9 - 24, The exported icon module paths use PascalCase (e.g., './CheckCircleFill', './MarkAsArchivedRead', './SlackColored'); rename the corresponding icon files to lowercase-dash names (e.g., check-circle-fill, mark-as-archived-read, slack-colored) and update the exports in packages/js/src/ui/icons/index.ts to export from the new lowercase-dash module paths (e.g., export * from './check-circle-fill') so the filenames and exports follow the repo's lowercase-with-dashes convention.packages/react/src/components/index.ts (1)
7-8: Public API expanded — plan a minor release for@novu/react.These new exports add consumer-facing surface area; please ensure release/versioning/changelog treats this as a semver minor update.
As per coding guidelines,
packages/**/*: Treat all exported symbols as public API; new exports should be released as minor versions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/index.ts` around lines 7 - 8, The new public exports SlackConnectButton and SlackLinkUser expand the consumer API surface for `@novu/react` and require a semver-minor release; update packages/react package.json to bump the minor version (or ensure your release tooling will publish a minor), add a changelog entry noting the new exported symbols SlackConnectButton and SlackLinkUser (and their source modules './slack-connect-button/SlackConnectButton' and './slack-link-user/SlackLinkUser'), and ensure CI/release configuration will publish this change as a minor release.packages/js/src/channel-connections/index.ts (1)
1-2: New SDK entrypoint exports look good; ensure semver-minor for@novu/js.Since these are new public exports, release planning should classify this as a minor version increment.
As per coding guidelines,
packages/**/*: New exports in packages should be treated as minor-version changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/channel-connections/index.ts` around lines 1 - 2, This adds new public exports (ChannelConnections from channel-connections and the types barrel) in packages/js/src/channel-connections/index.ts which constitutes a semver-minor change for the `@novu/js` package; update the release metadata to mark `@novu/js` as a minor release (e.g., add a changeset/manifest entry or bump release-type for the package to "minor" in your release configuration) so the new exports are published under a minor version, and reference the ChannelConnections export and the types barrel when creating the changeset.packages/js/src/channel-endpoints/index.ts (1)
2-2: Prefer explicit type exports in this public SDK barrel.Using
export *in a public package entrypoint can accidentally expose future internal additions as public API. Please export the intended types explicitly.Suggested refactor
export { ChannelEndpoints } from './channel-endpoints'; -export * from './types'; +export type { + CreateChannelEndpointArgs, + DeleteChannelEndpointArgs, + GetChannelEndpointArgs, + ListChannelEndpointsArgs, +} from './types';As per coding guidelines: "
packages/**/*: Treat all exported symbols in packages as public API; follow semver conventions."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/channel-endpoints/index.ts` at line 2, Replace the wildcard re-export in the public barrel (the current "export * from './types'") with explicit named exports to avoid accidentally exposing internal symbols; open the './types' module, identify the intended public types/interfaces (e.g., Channel, ChannelListParams, ChannelCreateParams — whatever actual names exist) and add a single-line explicit export statement exporting only those identifiers from './types' (remove the export *). Ensure the barrel only lists the exact type names you want to commit to as public API.packages/js/src/novu.ts (1)
24-25: Please confirm release metadata reflects this new public API surface.
channelConnectionsandchannelEndpointsare new public members onNovu; make sure this ships as a minor-version API addition with changelog coverage.As per coding guidelines: "
packages/**/*: Treat all exported symbols in packages as public API; follow semver conventions."Also applies to: 94-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/novu.ts` around lines 24 - 25, Update the package release metadata to reflect the new public API: add a changelog entry noting that Novu now exposes channelConnections and channelEndpoints as public members, bump the package version according to semver (minor), and include these symbols in any public API docs or export listings for the package; also ensure any other new public members added to the Novu class in this PR are documented in the changelog and release notes before cutting the release.apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.command.ts (1)
6-6: Keep the command layer independent from the usecase module.This command now depends on
generate-slack-oauth-url.usecasefor a shared type, which inverts the CQRS dependency direction. ExtractOAuthModeto a small sibling types module and import it from both places instead.As per coding guidelines,
apps/api/**: Ensure CQRS patterns (commands/queries) are followed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.command.ts` at line 6, The command file import creates an inverted CQRS dependency by importing OAuthMode from generate-slack-oauth-url.usecase; extract the shared type into a small sibling types module (e.g., create generate-slack-oauth-url.types.ts exporting OAuthMode) and update both generate-slack-oauth-url.command.ts and generate-slack-oauth-url.usecase.ts to import OAuthMode from that new module so the command layer no longer depends on the usecase module.packages/js/src/ui/components/Renderer.tsx (1)
158-180: Collapse the extra portal renderer into a shared helper.
ChannelComponentsRendereris a copy ofSubscriptionComponentsRenderer. Keeping both in sync will be easy to miss the next time the wrapper or portal logic changes. A small generic portal renderer would remove the duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/Renderer.tsx` around lines 158 - 180, Extract the duplicated portal rendering logic in ChannelComponentsRenderer and SubscriptionComponentsRenderer into a single reusable function/component (e.g., PortalComponentsRenderer) that accepts props { elements: MountableElement[]; nodes: Map<MountableElement, NovuComponent> } and internally uses Show, For, Portal and Root to mount the corresponding novuComponents by name and props (reuse the existing logic that resolves novuComponent via props.nodes.get(node) and looks up Component via novuComponents[novuComponent().name]). Replace both ChannelComponentsRenderer and SubscriptionComponentsRenderer bodies to call this new PortalComponentsRenderer (or thin wrappers) so there's one canonical implementation to maintain; update any imports/exports and call sites to use the new helper.playground/nextjs/src/pages/connect-chat/index.tsx (2)
9-10: Remove dead code.The
contextvariable is alwaysundefined(line 10), making the conditional spread at line 65 (...(context && { context: context })) dead code. Consider removing the commented line and the conditional spread, or uncommenting if context support is intended.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/nextjs/src/pages/connect-chat/index.tsx` around lines 9 - 10, The variable context is left as undefined and the conditional spread ...(context && { context: context }) is effectively dead code; remove the unused context declaration and delete the conditional spread in the payload construction (or if context support is intended, restore a meaningful value to the context variable and ensure it's passed correctly). Locate the const context declaration and the conditional spread expression in the component (look for "const context" and "...(context && { context: context })") and either remove both or replace the declaration with a real context object so the spread is reachable.
95-146: Consider using a single NovuProvider.Multiple
NovuProviderinstances wrap individual components (lines 95-116, 118-146, 156-166, 168-194). While this may be intentional for demonstrating isolated state, it's atypical and could confuse developers referencing this playground. If isolation isn't required, wrapping all components in a single provider would be more representative of real-world usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/nextjs/src/pages/connect-chat/index.tsx` around lines 95 - 146, The code currently creates multiple NovuProvider instances around separate SlackConnectButton examples (NovuProvider, SlackConnectButton, novuConfig); consolidate them by wrapping all SlackConnectButton components with a single NovuProvider that receives novuConfig to better represent real-world usage and avoid duplicated provider state—remove the extra NovuProvider wrappers and ensure each SlackConnectButton remains as-is (preserving their appearance/props like integrationIdentifier, connectLabel, connectedLabel, and custom icons) inside the single provider.packages/js/src/ui/api/hooks/useChannelEndpoint.ts (1)
92-93: Add blank line beforereturnstatement.As per coding guidelines: "Include a blank line before every
returnstatement."Proposed fix
createEffect(() => { setLoading(endpoint.loading); }); + return { endpoint, loading, mutate, refetch, create, remove }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/api/hooks/useChannelEndpoint.ts` around lines 92 - 93, The file's function useChannelEndpoint ends its body with "return { endpoint, loading, mutate, refetch, create, remove };" but lacks a blank line before the return; add a single blank line immediately above that return statement in the useChannelEndpoint function to follow the coding guideline (ensure no other code or comments are inserted between the blank line and the return).packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx (1)
153-154: Add blank line beforereturnstatement.As per coding guidelines: "Include a blank line before every
returnstatement."Proposed fix
} }; + return ( <div🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx` around lines 153 - 154, The return statement in the SlackLinkUser component needs a blank line before it per the coding guidelines; open the SlackLinkUser function/component and insert a single empty line immediately before the line that begins "return (" so there is a blank line separating preceding logic from the return, ensuring consistency with the "blank line before every return" rule.packages/js/src/ui/components/connect-chat/ConnectChat.tsx (1)
77-78: Add blank line beforereturnstatement.As per coding guidelines: "Include a blank line before every
returnstatement."Proposed fix
} }; + return ( <div🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/connect-chat/ConnectChat.tsx` around lines 77 - 78, The ConnectChat component's JSX return currently lacks the required blank line before the return statement; update the ConnectChat function (component) so there is a single blank line immediately preceding the "return (" line to follow the coding guideline "Include a blank line before every return statement" and keep surrounding whitespace consistent with other components.playground/nextjs/src/lib/slack-dm-endpoint-connect.ts (1)
134-138: Type assertion onep.endpointmay fail for unexpected structures.The assertion
ep.endpoint as Record<string, string>assumes all endpoints have string values. If an endpoint has different value types, theuserIdcomparison could behave unexpectedly. Consider using optional chaining or a type guard.Proposed defensive fix
const alreadyLinked = endpoints.result.data.some((ep) => { - const endpointData = ep.endpoint as Record<string, string>; - - return ep.type === SLACK_USER_TYPE && endpointData.userId === slackUserId; + if (ep.type !== SLACK_USER_TYPE) return false; + const endpointData = ep.endpoint; + + return typeof endpointData === 'object' && endpointData !== null && endpointData.userId === slackUserId; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/nextjs/src/lib/slack-dm-endpoint-connect.ts` around lines 134 - 138, The check that computes alreadyLinked assumes ep.endpoint is Record<string,string>; replace that unsafe assertion in the alreadyLinked computation by defensively reading ep.endpoint (from endpoints.result.data) with optional chaining and a type guard: ensure ep.type === SLACK_USER_TYPE and that ep.endpoint exists and its userId property is a string (e.g., typeof endpoint.userId === "string") before comparing to slackUserId, so unexpected shapes or non-string values won't cause incorrect matches or runtime errors.apps/api/src/app/inbox/inbox.controller.ts (1)
855-871: Type casting bypasses compile-time safety.The casts on lines 857 and 870 disable TypeScript's type checking. While the comment explains the discriminated union challenge, the
as Parameters<...>[0]cast on line 870 could hide mismatches ifCreateChannelEndpointCommandandCreateChannelEndpointRequesttypes diverge in the future.Consider defining a shared type or using a type-safe mapper function to ensure the DTO fields align with the command fields.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/inbox/inbox.controller.ts` around lines 855 - 871, The code is using unsafe casts (typedBody as Extract<CreateChannelEndpointRequest,...> and as Parameters<typeof CreateChannelEndpointCommand.create>[0]) which bypass compile-time checks; replace these casts by creating a small type-safe mapper or shared DTO that maps CreateChannelEndpointRequest -> CreateChannelEndpointCommand payload, explicitly picking and validating fields (identifier, integrationIdentifier, connectionIdentifier, context, type, endpoint) and return the correct shape for CreateChannelEndpointCommand.create; update the handler to call createChannelEndpointUsecase.execute(CreateChannelEndpointCommand.create(mappedPayload)) and remove the as-casts so the compiler enforces field compatibility between CreateChannelEndpointRequest and CreateChannelEndpointCommand.create payload.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de6f6842-7057-4f27-8966-68a709063e6b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (67)
apps/api/src/app/channel-connections/channel-connections.controller.tsapps/api/src/app/channel-connections/dtos/create-channel-connection-request.dto.tsapps/api/src/app/channel-connections/usecases/create-channel-connection/create-channel-connection.command.tsapps/api/src/app/channel-connections/usecases/create-channel-connection/create-channel-connection.usecase.tsapps/api/src/app/inbox/inbox.controller.tsapps/api/src/app/inbox/inbox.module.tsapps/api/src/app/integrations/dtos/generate-chat-oauth-url.dto.tsapps/api/src/app/integrations/integrations.controller.tsapps/api/src/app/integrations/usecases/chat-oauth-callback/slack-oauth-callback/slack-oauth-callback.usecase.tsapps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-chat-oauth-url.command.tsapps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-chat-oauth-url.usecase.tsapps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.command.tsapps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.tspackages/js/scripts/size-limit.mjspackages/js/src/api/inbox-service.tspackages/js/src/channel-connections/channel-connections.tspackages/js/src/channel-connections/helpers.tspackages/js/src/channel-connections/index.tspackages/js/src/channel-connections/types.tspackages/js/src/channel-endpoints/channel-endpoints.tspackages/js/src/channel-endpoints/helpers.tspackages/js/src/channel-endpoints/index.tspackages/js/src/channel-endpoints/types.tspackages/js/src/event-emitter/types.tspackages/js/src/index.tspackages/js/src/novu.tspackages/js/src/ui/api/hooks/useChannelConnection.tspackages/js/src/ui/api/hooks/useChannelEndpoint.tspackages/js/src/ui/components/Renderer.tsxpackages/js/src/ui/components/connect-chat/ConnectChat.tsxpackages/js/src/ui/components/index.tspackages/js/src/ui/components/link-user/LinkUser.tsxpackages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsxpackages/js/src/ui/components/slack-constants.tspackages/js/src/ui/components/slack-link-user/SlackLinkUser.tsxpackages/js/src/ui/config/appearanceKeys.tspackages/js/src/ui/icons/CheckCircleFill.tsxpackages/js/src/ui/icons/SlackColored.tsxpackages/js/src/ui/icons/index.tspackages/js/src/ui/index.tspackages/js/src/ui/types.tspackages/nextjs/src/app-router/index.tspackages/nextjs/src/pages-router/index.tspackages/react/src/components/NovuUI.tsxpackages/react/src/components/connect-chat/ConnectChat.tsxpackages/react/src/components/connect-chat/DefaultConnectChat.tsxpackages/react/src/components/index.tspackages/react/src/components/slack-connect-button/DefaultSlackConnectButton.tsxpackages/react/src/components/slack-connect-button/SlackConnectButton.tsxpackages/react/src/components/slack-link-user/DefaultSlackLinkUser.tsxpackages/react/src/components/slack-link-user/SlackLinkUser.tsxpackages/react/src/hooks/index.tspackages/react/src/hooks/useChannelConnection.tspackages/react/src/hooks/useChannelConnections.tspackages/react/src/hooks/useChannelEndpoint.tspackages/react/src/hooks/useCreateChannelEndpoint.tspackages/react/src/hooks/useDeleteChannelEndpoint.tspackages/react/src/index.tspackages/react/src/utils/appearance.tspackages/shared/src/types/channel-connection.tsplayground/nextjs/.env.exampleplayground/nextjs/package.jsonplayground/nextjs/src/components/SideNav.tsxplayground/nextjs/src/lib/slack-dm-endpoint-connect.tsplayground/nextjs/src/pages/api/slack-dm-endpoint.tsplayground/nextjs/src/pages/api/trigger-event.tsplayground/nextjs/src/pages/connect-chat/index.tsx
| const isSharedMode = stateData.connectionMode === 'shared'; | ||
| const connection = await this.createChannelConnection.execute( | ||
| CreateChannelConnectionCommand.create({ | ||
| identifier: stateData.identifier, | ||
| organizationId: stateData.organizationId, | ||
| environmentId: stateData.environmentId, | ||
| integrationIdentifier: integration.identifier, | ||
| subscriberId: stateData.subscriberId, | ||
| subscriberId: isSharedMode ? undefined : stateData.subscriberId, | ||
| context: stateData.context, | ||
| connectionMode: stateData.connectionMode, |
There was a problem hiding this comment.
Fail closed when a subscriber-scoped connect is missing subscriberId.
When connectionMode is absent or anything other than 'shared', this branch still allows subscriberId to be undefined and passes it into connection creation. That makes an unscoped connection possible unless a downstream guard happens to reject it.
🛡️ Suggested guard
const isSharedMode = stateData.connectionMode === 'shared';
+ if (!isSharedMode && !stateData.subscriberId) {
+ throw new BadRequestException('subscriberId is required unless connectionMode is shared');
+ }
const connection = await this.createChannelConnection.execute(As per coding guidelines, apps/api/**: Review with focus on security, authentication, and authorization.
📝 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 isSharedMode = stateData.connectionMode === 'shared'; | |
| const connection = await this.createChannelConnection.execute( | |
| CreateChannelConnectionCommand.create({ | |
| identifier: stateData.identifier, | |
| organizationId: stateData.organizationId, | |
| environmentId: stateData.environmentId, | |
| integrationIdentifier: integration.identifier, | |
| subscriberId: stateData.subscriberId, | |
| subscriberId: isSharedMode ? undefined : stateData.subscriberId, | |
| context: stateData.context, | |
| connectionMode: stateData.connectionMode, | |
| const isSharedMode = stateData.connectionMode === 'shared'; | |
| if (!isSharedMode && !stateData.subscriberId) { | |
| throw new BadRequestException('subscriberId is required unless connectionMode is shared'); | |
| } | |
| const connection = await this.createChannelConnection.execute( | |
| CreateChannelConnectionCommand.create({ | |
| identifier: stateData.identifier, | |
| organizationId: stateData.organizationId, | |
| environmentId: stateData.environmentId, | |
| integrationIdentifier: integration.identifier, | |
| subscriberId: isSharedMode ? undefined : stateData.subscriberId, | |
| context: stateData.context, | |
| connectionMode: stateData.connectionMode, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/api/src/app/integrations/usecases/chat-oauth-callback/slack-oauth-callback/slack-oauth-callback.usecase.ts`
around lines 64 - 73, The code allows creating a subscriber-scoped connection
without a subscriberId when connectionMode !== 'shared'; update the guard around
isSharedMode/createChannelConnection.execute so that when
stateData.connectionMode is not 'shared' you validate stateData.subscriberId is
present and otherwise fail-closed (throw or return an authorization/validation
error) before calling CreateChannelConnectionCommand.create; reference the
isSharedMode variable, stateData.subscriberId, stateData.connectionMode, the
createChannelConnection.execute call and CreateChannelConnectionCommand.create
when adding this check.
| userScope: command.userScope, | ||
| mode: command.mode, | ||
| connectionMode: command.connectionMode, |
There was a problem hiding this comment.
Reject provider-specific OAuth fields when provider is not Slack/Novu.
userScope, mode, and connectionMode are now forwarded for Slack/Novu, but effectively ignored in the MsTeams path. Please fail fast for unsupported combinations instead of silently dropping caller intent.
💡 Suggested guard
async execute(command: GenerateChatOauthUrlCommand): Promise<string> {
const integration = await this.getIntegration(command);
+
+ if (
+ integration.providerId === ChatProviderIdEnum.MsTeams &&
+ (command.userScope || command.mode || command.connectionMode)
+ ) {
+ throw new BadRequestException(
+ 'userScope, mode, and connectionMode are not supported for MsTeams OAuth URL generation'
+ );
+ }
switch (integration.providerId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-chat-oauth-url.usecase.ts`
around lines 33 - 35, The code currently forwards userScope, mode, and
connectionMode regardless of provider; update the validation in
GenerateChatOAuthUrlUseCase (the execute/handler that builds the OAuth command)
to reject requests that include any of userScope, mode, or connectionMode when
provider is not "slack" or "novu" — perform a guard early (before constructing
the provider-specific payload) and throw a clear validation error (e.g.,
BadRequest/InvalidArgument) identifying the unsupported fields so callers fail
fast instead of silently dropping intent.
| } catch (error) { | ||
| emitter.emit('channel-connection.oauth-url.resolved', { args, error }); | ||
|
|
||
| return { error: new NovuError('Failed to generate chat OAuth URL', error) }; |
There was a problem hiding this comment.
Emit the normalized NovuError on the event bus too.
These helpers return wrapped NovuErrors, but the corresponding *.resolved events broadcast the raw caught error. Event subscribers — including the new React hooks — can therefore receive a different error shape than the public SDK methods expose.
🐛 Suggested fix pattern
} catch (error) {
- emitter.emit('channel-connection.get.resolved', { args, error });
-
- return { error: new NovuError('Failed to get channel connection', error) };
+ const wrappedError = new NovuError('Failed to get channel connection', error);
+ emitter.emit('channel-connection.get.resolved', { args, error: wrappedError });
+
+ return { error: wrappedError };
}Apply the same pattern to each catch block in this file.
Also applies to: 51-54, 73-76, 95-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/js/src/channel-connections/helpers.ts` around lines 28 - 31, The
catch blocks currently emit the raw caught error on the event bus but return a
wrapped NovuError; change each emitter.emit call (e.g., the one emitting
'channel-connection.oauth-url.resolved' and the other emits at the other catch
sites) to emit the normalized NovuError instance instead of the raw error so
subscribers see the same error shape as the SDK methods—construct the NovuError
(new NovuError('Failed to generate chat OAuth URL', error)) once in the catch,
assign it to a variable (e.g., normalizedError) and use that variable both in
emitter.emit(...) payload and in the returned { error: normalizedError }, and
apply this pattern to all catch blocks in this file (the three other catch sites
referenced).
| @@ -0,0 +1,64 @@ | |||
| import { InboxService } from '../api'; | |||
| import { BaseModule } from '../base-module'; | |||
| import type { ChannelEndpointResponse } from '../channel-connections/types'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check where ChannelEndpointResponse is defined and used
rg -n 'ChannelEndpointResponse' packages/js/src/channel-endpoints/ packages/js/src/channel-connections/Repository: novuhq/novu
Length of output: 1104
🏁 Script executed:
# Check the type definition and context
cat -n packages/js/src/channel-connections/types.ts | head -30Repository: novuhq/novu
Length of output: 950
🏁 Script executed:
# Check if there are any local types in channel-endpoints module
find packages/js/src/channel-endpoints -name "*.ts" -o -name "*.tsx" | xargs grep -l "^export type\|^export interface" 2>/dev/null || echo "No type exports found"Repository: novuhq/novu
Length of output: 97
🏁 Script executed:
# Check if channel-endpoints has its own types file
ls -la packages/js/src/channel-endpoints/ 2>/dev/null | grep -E "types\.|index"Repository: novuhq/novu
Length of output: 172
🏁 Script executed:
# Check module structure to understand the relationship
ls -la packages/js/src/channel-connections/ | head -20Repository: novuhq/novu
Length of output: 420
🏁 Script executed:
# Check what's in the channel-endpoints/types.ts file
cat -n packages/js/src/channel-endpoints/types.tsRepository: novuhq/novu
Length of output: 261
🏁 Script executed:
# Also check the channel-endpoints index to understand the module structure
cat -n packages/js/src/channel-endpoints/index.tsRepository: novuhq/novu
Length of output: 149
Move ChannelEndpointResponse to channel-endpoints/types.ts for proper module cohesion and public API clarity.
ChannelEndpointResponse is semantically a response type for the endpoints module, but it's defined in and must be imported from channel-connections/types. The channel-endpoints/types.ts file already exists and re-exports request argument types from channel-connections/types, but omits the response type. For API clarity and module boundary integrity, either move ChannelEndpointResponse to channel-endpoints/types.ts or explicitly re-export it there so consumers of the channel-endpoints module have a consistent import path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/js/src/channel-endpoints/channel-endpoints.ts` at line 3, The
ChannelEndpointResponse type currently imported from channel-connections/types
should be relocated or re-exported from channel-endpoints/types.ts so consumers
import it from the channel-endpoints module; update channel-endpoints/types.ts
to either (a) move the ChannelEndpointResponse definition into that file and
adjust imports in channel-endpoints/channel-endpoints.ts to import
ChannelEndpointResponse from './types', or (b) add an explicit re-export line in
channel-endpoints/types.ts that exports ChannelEndpointResponse from
'channel-connections/types', then update channel-endpoints/channel-endpoints.ts
to import ChannelEndpointResponse from './types' to keep the public API cohesive
and consistent.
| } catch (error) { | ||
| emitter.emit('channel-endpoints.list.resolved', { args, error }); | ||
|
|
||
| return { error: new NovuError('Failed to list channel endpoints', error) }; |
There was a problem hiding this comment.
Emit the normalized NovuError on the event bus too.
These helpers return wrapped NovuErrors, but the corresponding *.resolved events broadcast the raw caught error. Event subscribers — including the new React hooks — can therefore receive a different error shape than the public SDK methods expose.
🐛 Suggested fix pattern
} catch (error) {
- emitter.emit('channel-endpoint.get.resolved', { args, error });
-
- return { error: new NovuError('Failed to get channel endpoint', error) };
+ const wrappedError = new NovuError('Failed to get channel endpoint', error);
+ emitter.emit('channel-endpoint.get.resolved', { args, error: wrappedError });
+
+ return { error: wrappedError };
}Apply the same pattern to each catch block in this file.
Also applies to: 51-54, 73-76, 95-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/js/src/channel-endpoints/helpers.ts` around lines 29 - 32, The catch
blocks in helpers.ts currently emit the raw caught error but return a wrapped
NovuError; change each catch to create the normalized error (e.g., const
normalized = new NovuError('Failed to list channel endpoints', error)) and use
that normalized error in both the emitter.emit call (replace the raw error) and
in the returned object; apply the same pattern to the other catch blocks that
emit '*.resolved' events so subscribers receive the same NovuError shape as the
public SDK methods.
| onMount(() => { | ||
| const currentNovu = novuAccessor(); | ||
|
|
||
| const cleanupDelete = currentNovu.on('channel-endpoint.delete.resolved', ({ args }) => { | ||
| if (args?.identifier && args.identifier === endpoint()?.identifier) { | ||
| setEndpoint(null); | ||
| } | ||
| }); | ||
|
|
||
| return () => { | ||
| cleanupDelete(); | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Memory leak: onMount return value is ignored in SolidJS.
SolidJS's onMount does not use the return value for cleanup (unlike React's useEffect). The cleanupDelete() function is never called, causing the event listener to persist after component unmount.
Proposed fix
onMount(() => {
const currentNovu = novuAccessor();
const cleanupDelete = currentNovu.on('channel-endpoint.delete.resolved', ({ args }) => {
if (args?.identifier && args.identifier === endpoint()?.identifier) {
setEndpoint(null);
}
});
- return () => {
+ onCleanup(() => {
cleanupDelete();
- };
+ });
});📝 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.
| onMount(() => { | |
| const currentNovu = novuAccessor(); | |
| const cleanupDelete = currentNovu.on('channel-endpoint.delete.resolved', ({ args }) => { | |
| if (args?.identifier && args.identifier === endpoint()?.identifier) { | |
| setEndpoint(null); | |
| } | |
| }); | |
| return () => { | |
| cleanupDelete(); | |
| }; | |
| }); | |
| onMount(() => { | |
| const currentNovu = novuAccessor(); | |
| const cleanupDelete = currentNovu.on('channel-endpoint.delete.resolved', ({ args }) => { | |
| if (args?.identifier && args.identifier === endpoint()?.identifier) { | |
| setEndpoint(null); | |
| } | |
| }); | |
| onCleanup(() => { | |
| cleanupDelete(); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx` around lines
66 - 78, The onMount callback currently returns cleanupDelete but SolidJS
ignores return values, so the listener from currentNovu.on (created in onMount
via novuAccessor and assigned to cleanupDelete) is never removed and leaks; fix
by importing and calling onCleanup from 'solid-js' inside the component (e.g.,
inside the same onMount block after creating cleanupDelete) to call
cleanupDelete() on unmount, ensuring setEndpoint and endpoint() logic remains
unchanged.
| const propsRef = useRef<UseChannelConnectionProps>(props); | ||
| propsRef.current = props; |
There was a problem hiding this comment.
Avoid wiring this hook to global channel-connection.get.* events as-is.
Those events are not scoped to props.identifier, so one get() elsewhere can flip isFetching and overwrite every useChannelConnection instance. Because the effect also ignores props.identifier, switching identifiers leaves stale state until manual refetch(), and this hook's own fetches are processed twice via both fetchConnection() and the event listener.
As per coding guidelines, packages/**/*: Treat all exported symbols in packages as public API; follow semver conventions with breaking changes requiring major bumps, new exports as minor versions, and fixes as patches.
Also applies to: 32-52, 57-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react/src/hooks/useChannelConnection.ts` around lines 24 - 25, The
hook useChannelConnection currently subscribes to global
"channel-connection.get.*" events and updates shared state via
propsRef/fetchConnection, which lets unrelated instances flip isFetching and
causes duplicate fetches and stale state when props.identifier changes; fix it
by scoping the event subscription to the instance identifier (either subscribe
to a namespaced event like `channel-connection.get.${props.identifier}` or check
an identifier field on the incoming event and ignore events whose identifier !==
propsRef.current.identifier), ensure the effect that sets up the listener
includes props.identifier so it cleans up and re-subscribes when identifier
changes, and avoid double-processing by only calling fetchConnection when the
event originates from a different source (use an optional sourceId flag in the
event or set a local flag) while keeping proper cleanup of the listener in the
effect; touch the useChannelConnection hook, propsRef usage, and fetchConnection
invocation to implement this scoping and de-duplication.
| const propsRef = useRef<UseChannelEndpointProps>(props); | ||
| propsRef.current = props; |
There was a problem hiding this comment.
Scope endpoint events to the tracked identifier.
channel-endpoint.get.* and especially channel-endpoint.create.resolved are global. As written, a fetch/create for another endpoint can overwrite this hook with unrelated data. The effect also ignores props.identifier, so changing identifiers leaves stale state, and the hook double-processes its own fetches through both fetchEndpoint() and the event listeners.
As per coding guidelines, packages/**/*: Treat all exported symbols in packages as public API; follow semver conventions with breaking changes requiring major bumps, new exports as minor versions, and fixes as patches.
Also applies to: 32-52, 57-99
| export default async function handler(req: NextApiRequest, res: NextApiResponse<ResponseData>) { | ||
| if (req.method !== 'POST') { | ||
| res.setHeader('Allow', 'POST'); | ||
| res.status(405).json({ error: 'Method not allowed' }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const body = req.body as RequestBody; | ||
| const subscriberId = typeof body.subscriberId === 'string' ? body.subscriberId.trim() : ''; | ||
|
|
||
| if (!subscriberId) { | ||
| res.status(400).json({ error: 'subscriberId is required' }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const integrationIdentifier = | ||
| (typeof body.integrationIdentifier === 'string' && body.integrationIdentifier.trim()) || | ||
| process.env.NOVU_SLACK_INTEGRATION_IDENTIFIER; | ||
|
|
||
| if (!integrationIdentifier) { | ||
| res.status(400).json({ error: 'integrationIdentifier is required (body or NOVU_SLACK_INTEGRATION_IDENTIFIER)' }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const result = await ensureSlackUserDmEndpoint({ | ||
| subscriberId, | ||
| integrationIdentifier, | ||
| emailOverride: typeof body.emailOverride === 'string' ? body.emailOverride : undefined, | ||
| slackUserIdOverride: typeof body.slackUserIdOverride === 'string' ? body.slackUserIdOverride : undefined, | ||
| }); | ||
|
|
||
| if (!result.ok) { | ||
| res.status(422).json({ error: result.error }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| res.status(200).json({ slackUserId: result.slackUserId }); | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : 'Unknown error'; | ||
|
|
||
| res.status(500).json({ error: message }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add authorization to this endpoint before running DM endpoint provisioning.
The route performs privileged server-side Slack/Novu actions but is currently callable without authentication. Please gate it (API key/session) before reading and processing the body.
Suggested patch (API-key gate example)
export default async function handler(req: NextApiRequest, res: NextApiResponse<ResponseData>) {
if (req.method !== 'POST') {
@@
return;
}
+
+ const expectedApiKey = process.env.PLAYGROUND_API_KEY?.trim();
+ const providedApiKeyHeader = req.headers['x-playground-api-key'];
+ const providedApiKey = Array.isArray(providedApiKeyHeader) ? providedApiKeyHeader[0] : providedApiKeyHeader;
+
+ if (!expectedApiKey || providedApiKey !== expectedApiKey) {
+ res.status(401).json({ error: 'Unauthorized' });
+
+ return;
+ }
try {📝 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.
| export default async function handler(req: NextApiRequest, res: NextApiResponse<ResponseData>) { | |
| if (req.method !== 'POST') { | |
| res.setHeader('Allow', 'POST'); | |
| res.status(405).json({ error: 'Method not allowed' }); | |
| return; | |
| } | |
| try { | |
| const body = req.body as RequestBody; | |
| const subscriberId = typeof body.subscriberId === 'string' ? body.subscriberId.trim() : ''; | |
| if (!subscriberId) { | |
| res.status(400).json({ error: 'subscriberId is required' }); | |
| return; | |
| } | |
| const integrationIdentifier = | |
| (typeof body.integrationIdentifier === 'string' && body.integrationIdentifier.trim()) || | |
| process.env.NOVU_SLACK_INTEGRATION_IDENTIFIER; | |
| if (!integrationIdentifier) { | |
| res.status(400).json({ error: 'integrationIdentifier is required (body or NOVU_SLACK_INTEGRATION_IDENTIFIER)' }); | |
| return; | |
| } | |
| const result = await ensureSlackUserDmEndpoint({ | |
| subscriberId, | |
| integrationIdentifier, | |
| emailOverride: typeof body.emailOverride === 'string' ? body.emailOverride : undefined, | |
| slackUserIdOverride: typeof body.slackUserIdOverride === 'string' ? body.slackUserIdOverride : undefined, | |
| }); | |
| if (!result.ok) { | |
| res.status(422).json({ error: result.error }); | |
| return; | |
| } | |
| res.status(200).json({ slackUserId: result.slackUserId }); | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : 'Unknown error'; | |
| res.status(500).json({ error: message }); | |
| } | |
| } | |
| export default async function handler(req: NextApiRequest, res: NextApiResponse<ResponseData>) { | |
| if (req.method !== 'POST') { | |
| res.setHeader('Allow', 'POST'); | |
| res.status(405).json({ error: 'Method not allowed' }); | |
| return; | |
| } | |
| const expectedApiKey = process.env.PLAYGROUND_API_KEY?.trim(); | |
| const providedApiKeyHeader = req.headers['x-playground-api-key']; | |
| const providedApiKey = Array.isArray(providedApiKeyHeader) ? providedApiKeyHeader[0] : providedApiKeyHeader; | |
| if (!expectedApiKey || providedApiKey !== expectedApiKey) { | |
| res.status(401).json({ error: 'Unauthorized' }); | |
| return; | |
| } | |
| try { | |
| const body = req.body as RequestBody; | |
| const subscriberId = typeof body.subscriberId === 'string' ? body.subscriberId.trim() : ''; | |
| if (!subscriberId) { | |
| res.status(400).json({ error: 'subscriberId is required' }); | |
| return; | |
| } | |
| const integrationIdentifier = | |
| (typeof body.integrationIdentifier === 'string' && body.integrationIdentifier.trim()) || | |
| process.env.NOVU_SLACK_INTEGRATION_IDENTIFIER; | |
| if (!integrationIdentifier) { | |
| res.status(400).json({ error: 'integrationIdentifier is required (body or NOVU_SLACK_INTEGRATION_IDENTIFIER)' }); | |
| return; | |
| } | |
| const result = await ensureSlackUserDmEndpoint({ | |
| subscriberId, | |
| integrationIdentifier, | |
| emailOverride: typeof body.emailOverride === 'string' ? body.emailOverride : undefined, | |
| slackUserIdOverride: typeof body.slackUserIdOverride === 'string' ? body.slackUserIdOverride : undefined, | |
| }); | |
| if (!result.ok) { | |
| res.status(422).json({ error: result.error }); | |
| return; | |
| } | |
| res.status(200).json({ slackUserId: result.slackUserId }); | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : 'Unknown error'; | |
| res.status(500).json({ error: message }); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@playground/nextjs/src/pages/api/slack-dm-endpoint.ts` around lines 33 - 80,
The handler function is unprotected and must validate caller credentials before
processing the request; add an authorization gate at the start of export default
async function handler (before reading req.body or calling
ensureSlackUserDmEndpoint) that checks for a valid API key or session (e.g.,
inspect req.headers.authorization or session cookie), verify it against your
server-side secret/verification function, and if invalid return 401/403 with an
error and stop further processing; ensure this check is applied to the POST path
only and include the same guard for any future callers of
ensureSlackUserDmEndpoint invoked from this route.
| export default async function handler(req: NextApiRequest, res: NextApiResponse<ResponseData>) { | ||
| if (req.method !== 'POST') { | ||
| res.setHeader('Allow', 'POST'); | ||
| res.status(405).json({ error: 'Method not allowed' }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const secretKey = process.env.NOVU_SECRET_KEY?.trim(); | ||
|
|
||
| if (!secretKey) { | ||
| res.status(500).json({ error: 'NOVU_SECRET_KEY is not configured' }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const body = req.body as RequestBody; | ||
|
|
||
| if (!body.name) { | ||
| res.status(400).json({ error: 'name (workflow ID) is required' }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| if (!body.to) { | ||
| res.status(400).json({ error: 'to (subscriber) is required' }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const backendUrl = ( | ||
| process.env.NOVU_API_BASE_URL ?? | ||
| process.env.NEXT_PUBLIC_NOVU_BACKEND_URL ?? | ||
| 'https://api.novu.co' | ||
| ).replace(/\/+$/, ''); | ||
|
|
||
| try { | ||
| const upstream = await fetch(`${backendUrl}/v1/events/trigger`, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Authorization: `ApiKey ${secretKey}`, | ||
| }, | ||
| body: JSON.stringify(body), | ||
| }); | ||
|
|
||
| const data = (await upstream.json()) as ResponseData; | ||
|
|
||
| res.status(upstream.status).json(data); | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : 'Unknown error'; | ||
|
|
||
| res.status(500).json({ error: message }); | ||
| } |
There was a problem hiding this comment.
Protect this proxy route with server-side authorization before forwarding.
This handler currently allows unauthenticated callers to invoke Novu event triggers with your server secret key. Add an API key/session check (or restrict to internal use only) before processing the request.
Suggested patch (API-key gate example)
export default async function handler(req: NextApiRequest, res: NextApiResponse<ResponseData>) {
if (req.method !== 'POST') {
@@
return;
}
+
+ const expectedApiKey = process.env.PLAYGROUND_API_KEY?.trim();
+ const providedApiKeyHeader = req.headers['x-playground-api-key'];
+ const providedApiKey = Array.isArray(providedApiKeyHeader) ? providedApiKeyHeader[0] : providedApiKeyHeader;
+
+ if (!expectedApiKey || providedApiKey !== expectedApiKey) {
+ res.status(401).json({ error: 'Unauthorized' });
+
+ return;
+ }
const secretKey = process.env.NOVU_SECRET_KEY?.trim();📝 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.
| export default async function handler(req: NextApiRequest, res: NextApiResponse<ResponseData>) { | |
| if (req.method !== 'POST') { | |
| res.setHeader('Allow', 'POST'); | |
| res.status(405).json({ error: 'Method not allowed' }); | |
| return; | |
| } | |
| const secretKey = process.env.NOVU_SECRET_KEY?.trim(); | |
| if (!secretKey) { | |
| res.status(500).json({ error: 'NOVU_SECRET_KEY is not configured' }); | |
| return; | |
| } | |
| const body = req.body as RequestBody; | |
| if (!body.name) { | |
| res.status(400).json({ error: 'name (workflow ID) is required' }); | |
| return; | |
| } | |
| if (!body.to) { | |
| res.status(400).json({ error: 'to (subscriber) is required' }); | |
| return; | |
| } | |
| const backendUrl = ( | |
| process.env.NOVU_API_BASE_URL ?? | |
| process.env.NEXT_PUBLIC_NOVU_BACKEND_URL ?? | |
| 'https://api.novu.co' | |
| ).replace(/\/+$/, ''); | |
| try { | |
| const upstream = await fetch(`${backendUrl}/v1/events/trigger`, { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `ApiKey ${secretKey}`, | |
| }, | |
| body: JSON.stringify(body), | |
| }); | |
| const data = (await upstream.json()) as ResponseData; | |
| res.status(upstream.status).json(data); | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : 'Unknown error'; | |
| res.status(500).json({ error: message }); | |
| } | |
| export default async function handler(req: NextApiRequest, res: NextApiResponse<ResponseData>) { | |
| if (req.method !== 'POST') { | |
| res.setHeader('Allow', 'POST'); | |
| res.status(405).json({ error: 'Method not allowed' }); | |
| return; | |
| } | |
| const expectedApiKey = process.env.PLAYGROUND_API_KEY?.trim(); | |
| const providedApiKeyHeader = req.headers['x-playground-api-key']; | |
| const providedApiKey = Array.isArray(providedApiKeyHeader) ? providedApiKeyHeader[0] : providedApiKeyHeader; | |
| if (!expectedApiKey || providedApiKey !== expectedApiKey) { | |
| res.status(401).json({ error: 'Unauthorized' }); | |
| return; | |
| } | |
| const secretKey = process.env.NOVU_SECRET_KEY?.trim(); | |
| if (!secretKey) { | |
| res.status(500).json({ error: 'NOVU_SECRET_KEY is not configured' }); | |
| return; | |
| } | |
| const body = req.body as RequestBody; | |
| if (!body.name) { | |
| res.status(400).json({ error: 'name (workflow ID) is required' }); | |
| return; | |
| } | |
| if (!body.to) { | |
| res.status(400).json({ error: 'to (subscriber) is required' }); | |
| return; | |
| } | |
| const backendUrl = ( | |
| process.env.NOVU_API_BASE_URL ?? | |
| process.env.NEXT_PUBLIC_NOVU_BACKEND_URL ?? | |
| 'https://api.novu.co' | |
| ).replace(/\/+$/, ''); | |
| try { | |
| const upstream = await fetch(`${backendUrl}/v1/events/trigger`, { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `ApiKey ${secretKey}`, | |
| }, | |
| body: JSON.stringify(body), | |
| }); | |
| const data = (await upstream.json()) as ResponseData; | |
| res.status(upstream.status).json(data); | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : 'Unknown error'; | |
| res.status(500).json({ error: message }); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@playground/nextjs/src/pages/api/trigger-event.ts` around lines 22 - 75, The
handler currently forwards requests using process.env.NOVU_SECRET_KEY without
any auth; add a server-side authorization check at the top of the handler
(before reading body or calling the Novu API) by validating either a shared
internal API key (e.g., compare req.headers['x-internal-api-key'] to
process.env.INTERNAL_API_KEY) or a verified session/token, and return 401/403
immediately if the check fails; ensure this check occurs in the exported handler
function and short-circuits all subsequent logic that uses
NOVU_SECRET_KEY/request body.
1 participant
@coderabbitai summary
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer
Note
Medium Risk
Adds new authenticated subscriber endpoints and client SDK flows for creating/listing/deleting channel connections/endpoints and generating Slack OAuth URLs; mistakes could expose cross-subscriber resources or break inbox API compatibility. Gated behind the
IS_SLACK_TEAMS_ENABLEDfeature flag which limits blast radius but needs careful rollout/testing.Overview
Adds new subscriber-authenticated Inbox routes for
channel-connections,channel-endpoints, andchat/oauth, including per-subscriber access checks and feature-flag gating viaIS_SLACK_TEAMS_ENABLED.Extends
@novu/jswith newInboxServicemethods,ChannelConnections/ChannelEndpointsmodules, event-emitter events, and exports; updatesNovuto expose these modules.Adds UI components and hooks (
ConnectChat,LinkUser) in@novu/js/uiand React wrappers + hooks in@novu/react/@novu/nextjs, plus a Next.js playground page and server-side helpers/routes for Slack DM endpoint registration and triggering test events. Also bumps JS bundle size limit slightly.Reviewed by Cursor Bugbot for commit 1900527. Configure here.