feat(dashboard, api-service): Slack quickstart flow#10728
Merged
Conversation
Introduce a new ProviderDropdown component (apps/dashboard/src/components/agents/provider-dropdown.tsx) that renders a searchable, grouped provider picker (uses novu providers, existing integrations, Popover + Command UI). Replace the inline ProviderDropdown in AgentSetupGuide with the new component, add selectedProviderId state and pass value/onSelect to handle selection. The new picker groups by channel, shows existing integrations (or an add icon), and closes on select.
Make integration identifier optional in CreateIntegrationData and enhance Slack agent setup flow: generate a safe YAML manifest (escape strings), build a webhook URL that includes the integration identifier, and expand the Slack manifest with updated features, scopes, settings, and event subscriptions. Wire integrations state into AgentSetupGuide (useFetchIntegrations) to allow selecting an existing integration or showing a placeholder identifier when none is selected. Replace the simple provider selector with a more capable ProviderDropdown: support listing existing integrations, creating new integrations, linking them to an agent (mutations + query invalidation), show loading state and toasts, require an environment, and disable UI while operations are pending. Overall this enables creating/linking Slack integrations directly from the dropdown and produces a more complete Slack app manifest for agents.
Pass a Set of linked integration IDs from AgentSetupGuide to ProviderDropdown and use it to skip the addAgentIntegrationMutation when a user selects an integration that's already linked to the agent. Adds a memoized linkedIntegrationIds in AgentSetupGuide, a new optional prop (with comment) on ProviderDropdown, and a check to prevent duplicate API calls/toasts while still invoking onSelect and closing the dropdown.
Expose an 'active' boolean for agents across backend and frontend. API: add active to DTOs, commands, mappers and usecases (create defaults to true; update accepts active and requires at least one of name, description or active). Controller payloads updated. DAL agent entity/schema updated accordingly. Dashboard: propagate active into types and UI — sidebar shows Active/Inactive badge and a Switch to toggle active state. Agent integration flows were refactored to use integrationIdentifier (route & params) instead of providerId, adjust router path, and update resolve/guide components. Integrations sidebar now uses a ProviderDropdown (with excludeLinked and renderTrigger options) instead of the old sheet; it auto-redirects to the first linked integration unless skipped. Webhook URL generation now includes the integration identifier. Miscellaneous UI/UX and code cleanup (manifest section formatting, copy tweaks, memoization/useEffect improvements, and small component reorganizations).
agent-setup-guide: Simplify polling logic for agent integration status by removing refs and extra state, using local intervalId and confettiFired flags, and ensuring proper cleanup. Move confetti rendering into a createPortal with explicit dimensions and increased z-index, and remove focus-based celebration handling. provider-dropdown: Add detection for 409 "already linked" errors and gracefully handle race conditions when linking integrations by ignoring duplicate-link errors or selecting the existing integration. Centralize cache invalidation (using environmentId) and improve query refetch/invalidate flow to keep UI consistent after link operations.
Throw a specific NoBridgeUrlError when no bridge URL is configured and send an onboarding reply instead of silently returning. Added NoBridgeUrlError and changed BridgeExecutorService to throw it; ensure it is rethrown for non-bridge cases. AgentInboundHandler now catches NoBridgeUrlError and invokes HandleAgentReply to post an onboarding message (markdown) to the conversation. Introduced forwardRef/Inject wiring between ChatSdkService, AgentInboundHandler, and HandleAgentReply to resolve circular dependencies and adjusted imports and error handling accordingly.
✅ Deploy Preview for dashboard-v2-novu-staging ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
bot
added
@novu/dal
@novu/shared
@novu/dashboard
@novu/api-service
labels
Contributor
|
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: PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name |
commit: |
Contributor
📝 WalkthroughWalkthroughAdds agent Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Dashboard UI
participant ProvDD as ProviderDropdown
participant API as Dashboard API client
participant Backend as Agent API
participant Bridge as BridgeExecutor / ChatSdk
participant Inbound as AgentInboundHandler
User->>UI: Open Agent Setup
UI->>API: listIntegrations()
API-->>UI: integrations list
User->>ProvDD: Select provider/integration
ProvDD->>API: createIntegration? / addAgentIntegration
API->>Backend: POST /integrations or PATCH /agents/:id
Backend-->>API: integration created/linked
API-->>ProvDD: success
ProvDD-->>UI: selection confirmed
alt Slack selected and not connected
UI->>API: poll listAgentIntegrations (1s)
API->>Backend: GET /agents/:id/integrations
Backend-->>API: shows connectedAt when workspace connected
API-->>UI: connectedAt set
UI->>UI: render Connected + confetti
end
User->>UI: Trigger message delivery (via agent)
UI->>Backend: send message to agent -> BridgeExecutor
Backend->>Bridge: resolveBridgeUrl -> returns none
Bridge-->>Backend: throws NoBridgeUrlError
Backend->>Inbound: catch NoBridgeUrlError, call HandleAgentReply
Inbound->>Backend: persist system onboarding reply / respond to user
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
packages/shared/src/consts/providers/conversational-providers.ts (1)
3-20: Harden shared exports as readonly public API.
CONVERSATIONAL_PROVIDERSandCONVERSATIONAL_PROVIDER_IDSare mutable exports; consumers can mutate them globally at runtime. Consider exposing readonly shapes.♻️ Suggested refactor
-export type ConversationalProvider = { +export type ConversationalProvider = Readonly<{ providerId: string; displayName: string; comingSoon?: boolean; -}; +}>; -export const CONVERSATIONAL_PROVIDERS: ConversationalProvider[] = [ +export const CONVERSATIONAL_PROVIDERS = [ { providerId: ChatProviderIdEnum.Slack, displayName: 'Slack' }, { providerId: ChatProviderIdEnum.MsTeams, displayName: 'MS Teams', comingSoon: true }, { providerId: ChatProviderIdEnum.WhatsAppBusiness, displayName: 'WhatsApp Business', comingSoon: true }, { providerId: 'telegram', displayName: 'Telegram', comingSoon: true }, { providerId: 'google-chat', displayName: 'Google Chat', comingSoon: true }, { providerId: 'linear', displayName: 'Linear', comingSoon: true }, { providerId: 'zoom', displayName: 'Zoom', comingSoon: true }, { providerId: 'imessages', displayName: 'iMessages', comingSoon: true }, -]; +] as const satisfies readonly ConversationalProvider[]; -export const CONVERSATIONAL_PROVIDER_IDS = new Set(CONVERSATIONAL_PROVIDERS.map((p) => p.providerId)); +export const CONVERSATIONAL_PROVIDER_IDS: ReadonlySet<string> = new Set( + CONVERSATIONAL_PROVIDERS.map((p) => p.providerId) +);As per coding guidelines,
packages/sharedexports are public API and should avoid mutable cross-runtime side effects.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/consts/providers/conversational-providers.ts` around lines 3 - 20, Change the mutable public exports to readonly/immutable shapes: make the ConversationalProvider type properties readonly (e.g., readonly providerId; readonly displayName; readonly comingSoon?), export CONVERSATIONAL_PROVIDERS as a ReadonlyArray<ConversationalProvider> and create/freeze the array and each provider object (e.g., Object.freeze([...]) / Object.freeze(provider) for each entry) so consumers cannot mutate entries at runtime, and export CONVERSATIONAL_PROVIDER_IDS as a ReadonlySet<string> (create with new Set(...) then Object.freeze(...) to prevent reassignment) so both the list and id set are exposed as immutable public API; update declarations for CONVERSATIONAL_PROVIDERS and CONVERSATIONAL_PROVIDER_IDS accordingly and keep provider identifiers (e.g., ChatProviderIdEnum.Slack) unchanged.apps/api/src/app/agents/services/agent-credential.service.ts (1)
80-89: Side-effect inresolve()lacks error handling and may surprise callers.The
resolve()method was previously a read-only lookup but now mutates state by settingconnectedAt. Two concerns:
- Unhandled error: If
updateOnefails, the exception propagates and aborts the entire resolve flow, which could break webhook handling unexpectedly.- Method semantics: Callers (e.g.,
handleWebhook,sendMessage) may not expectresolve()to perform writes.Consider wrapping the update in a try-catch to log failures without blocking resolution, or document the side-effect clearly:
♻️ Proposed defensive handling
if (agentIntegration.connectedAt == null) { - await this.agentIntegrationRepository.updateOne( - { - _id: agentIntegration._id, - _environmentId: environmentId, - _organizationId: organizationId, - }, - { $set: { connectedAt: new Date() } } - ); + try { + await this.agentIntegrationRepository.updateOne( + { + _id: agentIntegration._id, + _environmentId: environmentId, + _organizationId: organizationId, + }, + { $set: { connectedAt: new Date() } } + ); + } catch (err) { + // Non-critical: log and continue — connectedAt can be set on next call + // Consider adding a logger injection to track these failures + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/services/agent-credential.service.ts` around lines 80 - 89, The resolve() method now mutates agentIntegration.connectedAt via agentIntegrationRepository.updateOne which can throw and unexpectedly break callers like handleWebhook or sendMessage; wrap the updateOne call that sets connectedAt in a try-catch, log the error (including agentIntegration._id/organizationId/environmentId) and swallow the exception so resolve() remains a non-blocking read operation, or alternatively expose a flag parameter on resolve() to toggle the side-effect so callers can opt-in; ensure you reference resolve(), connectedAt and agentIntegrationRepository.updateOne when making the change.apps/dashboard/src/components/agents/agent-sidebar-widget.tsx (1)
334-338: Consider usingaddSuffix: trueto simplify the last updated display.The current approach manually concatenates "ago" after calling
formatDistanceToNowwithaddSuffix: false. UsingaddSuffix: truewould produce the same result with less code.♻️ Suggested simplification
- <p className="text-label-xs font-medium"> - <span className="text-text-soft">Last updated </span> - <span className="text-text-sub">{formatDistanceToNow(new Date(agent.updatedAt), { addSuffix: false })}</span> - <span className="text-text-sub"> ago</span> - </p> + <p className="text-label-xs font-medium"> + <span className="text-text-soft">Last updated </span> + <span className="text-text-sub">{formatDistanceToNow(new Date(agent.updatedAt), { addSuffix: true })}</span> + </p>Note:
addSuffix: trueproduces "X minutes ago" directly. If you need the specific styling split between the time value and "ago", the current approach is intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/agents/agent-sidebar-widget.tsx` around lines 334 - 338, The JSX manually appends "ago" after calling formatDistanceToNow(new Date(agent.updatedAt), { addSuffix: false }); change the call to formatDistanceToNow(new Date(agent.updatedAt), { addSuffix: true }) and remove the separate " ago" span so the rendered string already includes the suffix; update the spans around the time value in the component (the element using formatDistanceToNow and the surrounding <p> / <span> with className "text-text-sub") accordingly.apps/dashboard/src/components/agents/provider-dropdown.tsx (1)
270-275: Consider removing redundantrefetchQueriesbeforeinvalidateAgentLinkQueries.Line 270 calls
refetchQueriesforfetchIntegrations, then line 275 callsinvalidateAgentLinkQuerieswhich also invalidatesfetchIntegrations(line 234). The invalidation will trigger a refetch anyway, making the explicitrefetchQueriescall redundant.♻️ Suggested simplification
const created = await createIntegrationMutation.mutateAsync({ providerId: item.providerId, name: uniqueName, }); await addAgentIntegrationMutation.mutateAsync(created.identifier); showSuccessToast('Integration linked', `${created.name} was added to this agent.`); - await queryClient.refetchQueries({ queryKey: [QueryKeys.fetchIntegrations, environmentId] }); onSelect(item.providerId, created); setOpen(false); + await invalidateAgentLinkQueries(); + } else { + // existing integration branch continues... } - - await invalidateAgentLinkQueries();Alternatively, keep both if the synchronous refetch before
onSelectis intentional for immediate UI update.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/agents/provider-dropdown.tsx` around lines 270 - 275, The explicit await queryClient.refetchQueries({ queryKey: [QueryKeys.fetchIntegrations, environmentId] }) is redundant because invalidateAgentLinkQueries already invalidates the same key (see invalidateAgentLinkQueries) and will trigger a refetch; remove that refetchQueries call, keep the onSelect(item.providerId, created) and setOpen(false) flow, and ensure any required immediate synchronous refresh is handled (only keep refetchQueries if you intentionally need the refetch to complete before calling onSelect).apps/dashboard/src/components/agents/slack-setup-guide.tsx (2)
108-109: Consider increasing the polling interval to reduce server load.A 1-second polling interval may be aggressive, especially if multiple users are on the setup page simultaneously. Consider using 2-3 seconds, or implementing exponential backoff.
♻️ Suggested adjustment
void tick(); - intervalId = setInterval(() => void tick(), 1000); + intervalId = setInterval(() => void tick(), 3000);Alternatively, consider using React Query's
refetchIntervaloption if converting to a proper query, which also handles background tab throttling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/agents/slack-setup-guide.tsx` around lines 108 - 109, The current polling setup calls void tick() immediately and sets intervalId = setInterval(() => void tick(), 1000), which uses a 1-second interval; change this to a less aggressive interval (e.g., 2000–3000 ms) or implement exponential backoff on the tick function so it increases delay after repeated unchanged responses; update the setInterval call (and any intervalId management/clear logic) to use the new interval variable or backoff mechanism, and optionally consider replacing this manual polling with React Query's refetchInterval on the query that tick() drives to benefit from built-in throttling and background-tab handling.
217-219: YAMLrequest_urlvalues should be quoted for safety.The
request_urlfields (lines 219, 231) are not wrapped in quotes, unlike other URL fields. While typically safe, if the webhook URL contains special YAML characters (like#), it could cause parsing issues.♻️ Suggested fix
settings: event_subscriptions: - request_url: ${webhookHandlerUrl} + request_url: "${escapeYamlDoubleQuoted(webhookHandlerUrl)}" bot_events: # ... interactivity: is_enabled: true - request_url: ${webhookHandlerUrl} + request_url: "${escapeYamlDoubleQuoted(webhookHandlerUrl)}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/agents/slack-setup-guide.tsx` around lines 217 - 219, The YAML snippet sets request_url using an unquoted interpolation (${webhookHandlerUrl}), which can break if the URL contains YAML-special characters; update the YAML in slack-setup-guide.tsx so the request_url fields use quoted values (e.g., wrap ${webhookHandlerUrl} in double quotes) for all occurrences of request_url in that YAML block to ensure safe parsing.apps/dashboard/src/components/agents/setup-guide-primitives.tsx (1)
19-24: Consider using Tailwind color tokens instead of hardcoded hex values.The completed step indicator uses hardcoded hex colors (
#5ec269,#77db89). Per coding guidelines, prefer Tailwind utility classes. If these are design-system colors, consider adding them to the Tailwind config.♻️ Example using existing tokens
- <div className="flex size-5 shrink-0 items-center justify-center rounded-full border border-[`#5ec269`] bg-[`#77db89`] shadow-[0px_0px_0px_1px_#FFF,0px_0px_0px_2px_#E1E4EA]"> + <div className="flex size-5 shrink-0 items-center justify-center rounded-full border border-success-base bg-success-base shadow-[0px_0px_0px_1px_#FFF,0px_0px_0px_2px_#E1E4EA]">If the specific green shades are intentional design choices, keeping the hex values is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/agents/setup-guide-primitives.tsx` around lines 19 - 24, The completed-step indicator is using hardcoded hex colors in the JSX element with className 'flex size-5 shrink-0 items-center justify-center rounded-full border border-[`#5ec269`] bg-[`#77db89`] shadow[...]'—replace those hex values with Tailwind utility color classes (or with a new design-system token you add to tailwind.config.js and then use its utility class) so the component uses theme colors instead of literals; update the div's className to use the appropriate border-<token> and bg-<token> classes (and keep the svg stroke as text-white or equivalent) and add the new color tokens to theme.extend.colors if they don’t already exist.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/agents/usecases/update-agent/update-agent.command.ts`:
- Around line 20-22: The UpdateAgentCommand allows null to bypass boolean
validation because `@IsOptional`() skips `@IsBoolean`() for null; replace
`@IsOptional`() on the active property with `@ValidateIf`((_, value) => value !==
undefined) so null values are validated by `@IsBoolean`(), and keep the existing
usecase check that tests command.active !== undefined unchanged; update the
decorator on the active property in UpdateAgentCommand to use ValidateIf to
ensure only undefined means "not provided".
In `@apps/api/src/app/agents/usecases/update-agent/update-agent.usecase.ts`:
- Around line 12-18: The validation error message in update-agent use case is
out of sync with the conditional: update the thrown BadRequestException in the
update logic (the block that checks command.name, command.description,
command.behavior, command.active) so the message includes "active" as an allowed
field (e.g., "...name, description, behavior, or active must be provided.");
locate the check in update-agent.usecase.ts (the function/method handling the
update command) and replace the old message text accordingly.
In `@apps/dashboard/src/components/agents/slack-setup-guide.tsx`:
- Around line 88-93: The setTimeout started when confetti is shown can fire
after the component unmounts; capture the timer id returned by window.setTimeout
(e.g., const timeoutId = window.setTimeout(...)) and store it in a ref (e.g.,
confettiTimeoutRef) or a variable in a useEffect scope, then clear it with
clearTimeout(timeoutId) in a cleanup function (useEffect return) and also clear
any existing timeout before creating a new one; update the code around
confettiFired, setShowConfetti, and the window.setTimeout call to use the
ref/timeout id and clear it on unmount to prevent calling setShowConfetti on an
unmounted component.
In `@libs/dal/src/repositories/agent/agent.schema.ts`:
- Around line 17-20: The Agent schema's active field (agent.schema.ts: property
"active") currently only sets default: true which won't protect existing
documents or prevent null-ish writes; update the schema to make active required
(e.g., add required: true) while keeping default: true to enforce validation on
future writes, and add a one-time backfill migration that finds Agent documents
with active === null or active not set and updates them to true (include the
migration in libs/dal with a clear id so ops can run it). Ensure the change is
accompanied by tests or a small script that verifies no documents remain without
active and that the schema change is communicated as a migration-required
change.
---
Nitpick comments:
In `@apps/api/src/app/agents/services/agent-credential.service.ts`:
- Around line 80-89: The resolve() method now mutates
agentIntegration.connectedAt via agentIntegrationRepository.updateOne which can
throw and unexpectedly break callers like handleWebhook or sendMessage; wrap the
updateOne call that sets connectedAt in a try-catch, log the error (including
agentIntegration._id/organizationId/environmentId) and swallow the exception so
resolve() remains a non-blocking read operation, or alternatively expose a flag
parameter on resolve() to toggle the side-effect so callers can opt-in; ensure
you reference resolve(), connectedAt and agentIntegrationRepository.updateOne
when making the change.
In `@apps/dashboard/src/components/agents/agent-sidebar-widget.tsx`:
- Around line 334-338: The JSX manually appends "ago" after calling
formatDistanceToNow(new Date(agent.updatedAt), { addSuffix: false }); change the
call to formatDistanceToNow(new Date(agent.updatedAt), { addSuffix: true }) and
remove the separate " ago" span so the rendered string already includes the
suffix; update the spans around the time value in the component (the element
using formatDistanceToNow and the surrounding <p> / <span> with className
"text-text-sub") accordingly.
In `@apps/dashboard/src/components/agents/provider-dropdown.tsx`:
- Around line 270-275: The explicit await queryClient.refetchQueries({ queryKey:
[QueryKeys.fetchIntegrations, environmentId] }) is redundant because
invalidateAgentLinkQueries already invalidates the same key (see
invalidateAgentLinkQueries) and will trigger a refetch; remove that
refetchQueries call, keep the onSelect(item.providerId, created) and
setOpen(false) flow, and ensure any required immediate synchronous refresh is
handled (only keep refetchQueries if you intentionally need the refetch to
complete before calling onSelect).
In `@apps/dashboard/src/components/agents/setup-guide-primitives.tsx`:
- Around line 19-24: The completed-step indicator is using hardcoded hex colors
in the JSX element with className 'flex size-5 shrink-0 items-center
justify-center rounded-full border border-[`#5ec269`] bg-[`#77db89`]
shadow[...]'—replace those hex values with Tailwind utility color classes (or
with a new design-system token you add to tailwind.config.js and then use its
utility class) so the component uses theme colors instead of literals; update
the div's className to use the appropriate border-<token> and bg-<token> classes
(and keep the svg stroke as text-white or equivalent) and add the new color
tokens to theme.extend.colors if they don’t already exist.
In `@apps/dashboard/src/components/agents/slack-setup-guide.tsx`:
- Around line 108-109: The current polling setup calls void tick() immediately
and sets intervalId = setInterval(() => void tick(), 1000), which uses a
1-second interval; change this to a less aggressive interval (e.g., 2000–3000
ms) or implement exponential backoff on the tick function so it increases delay
after repeated unchanged responses; update the setInterval call (and any
intervalId management/clear logic) to use the new interval variable or backoff
mechanism, and optionally consider replacing this manual polling with React
Query's refetchInterval on the query that tick() drives to benefit from built-in
throttling and background-tab handling.
- Around line 217-219: The YAML snippet sets request_url using an unquoted
interpolation (${webhookHandlerUrl}), which can break if the URL contains
YAML-special characters; update the YAML in slack-setup-guide.tsx so the
request_url fields use quoted values (e.g., wrap ${webhookHandlerUrl} in double
quotes) for all occurrences of request_url in that YAML block to ensure safe
parsing.
In `@packages/shared/src/consts/providers/conversational-providers.ts`:
- Around line 3-20: Change the mutable public exports to readonly/immutable
shapes: make the ConversationalProvider type properties readonly (e.g., readonly
providerId; readonly displayName; readonly comingSoon?), export
CONVERSATIONAL_PROVIDERS as a ReadonlyArray<ConversationalProvider> and
create/freeze the array and each provider object (e.g., Object.freeze([...]) /
Object.freeze(provider) for each entry) so consumers cannot mutate entries at
runtime, and export CONVERSATIONAL_PROVIDER_IDS as a ReadonlySet<string> (create
with new Set(...) then Object.freeze(...) to prevent reassignment) so both the
list and id set are exposed as immutable public API; update declarations for
CONVERSATIONAL_PROVIDERS and CONVERSATIONAL_PROVIDER_IDS accordingly and keep
provider identifiers (e.g., ChatProviderIdEnum.Slack) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: faac3439-0400-4c3b-b93f-b839bf5d94ca
⛔ Files ignored due to path filters (5)
apps/dashboard/public/images/providers/light/square/google-chat.svgis excluded by!**/*.svgapps/dashboard/public/images/providers/light/square/imessages.svgis excluded by!**/*.svgapps/dashboard/public/images/providers/light/square/linear.svgis excluded by!**/*.svgapps/dashboard/public/images/providers/light/square/telegram.svgis excluded by!**/*.svgapps/dashboard/public/images/providers/light/square/zoom.svgis excluded by!**/*.svg
📒 Files selected for processing (36)
apps/api/src/app/agents/agents.controller.tsapps/api/src/app/agents/dtos/agent-integration-response.dto.tsapps/api/src/app/agents/dtos/agent-response.dto.tsapps/api/src/app/agents/dtos/create-agent-request.dto.tsapps/api/src/app/agents/dtos/update-agent-request.dto.tsapps/api/src/app/agents/mappers/agent-response.mapper.tsapps/api/src/app/agents/services/agent-credential.service.tsapps/api/src/app/agents/services/agent-inbound-handler.service.tsapps/api/src/app/agents/services/bridge-executor.service.tsapps/api/src/app/agents/services/chat-sdk.service.tsapps/api/src/app/agents/usecases/create-agent/create-agent.command.tsapps/api/src/app/agents/usecases/create-agent/create-agent.usecase.tsapps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.usecase.tsapps/api/src/app/agents/usecases/update-agent/update-agent.command.tsapps/api/src/app/agents/usecases/update-agent/update-agent.usecase.tsapps/dashboard/src/api/agents.tsapps/dashboard/src/api/integrations.tsapps/dashboard/src/components/agents/agent-integration-guides/agent-integration-guide-layout.tsxapps/dashboard/src/components/agents/agent-integration-guides/resolve-agent-integration-guide.tsxapps/dashboard/src/components/agents/agent-integrations-tab.tsxapps/dashboard/src/components/agents/agent-overview-tab.tsxapps/dashboard/src/components/agents/agent-setup-guide.tsxapps/dashboard/src/components/agents/agent-sidebar-widget.tsxapps/dashboard/src/components/agents/provider-dropdown.tsxapps/dashboard/src/components/agents/setup-guide-primitives.tsxapps/dashboard/src/components/agents/setup-guide-step-utils.tsapps/dashboard/src/components/agents/slack-setup-guide.tsxapps/dashboard/src/main.tsxapps/dashboard/src/pages/agent-details.tsxapps/dashboard/src/utils/routes.tslibs/dal/src/repositories/agent-integration/agent-integration.entity.tslibs/dal/src/repositories/agent-integration/agent-integration.schema.tslibs/dal/src/repositories/agent/agent.entity.tslibs/dal/src/repositories/agent/agent.schema.tspackages/shared/src/consts/providers/conversational-providers.tspackages/shared/src/consts/providers/index.ts
Multiple fixes and refinements across API and dashboard: - API: Inject PinoLogger into AgentCredentialService for logging. Update UpdateAgentCommand to validate optional `active` explicitly and include `active` in the usecase's required-fields error message. - Dashboard (agents): Show relative updated time with suffix (e.g. "3 minutes ago"). - Provider dropdown: tighten types to readonly for conversational providers and remove an unnecessary query refetch after creating an integration. - Slack setup guide: manage confetti timeout via a ref to avoid multiple timers/leaks, ensure webhook URL is YAML-escaped and quoted in the generated Slack manifest, and add missing imports (useRef). - Shared: make conversational providers immutable (freeze objects and arrays) and export a frozen set of provider IDs. These changes improve robustness (timers, logging), correctness (validation, manifest quoting), and immutability for shared provider constants.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/dashboard/src/components/agents/setup-guide-primitives.tsx (1)
17-24: Match the repo’s blank-line-before-return rule.A few new
returnstatements still skip the required blank line (StepIndicator,SetupStep,SetupButton, and the early exits inIntegrationCredentialsSidebar). As per coding guidelines "Include a blank line before everyreturnstatement".Also applies to: 51-52, 103-104, 132-132, 156-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/agents/setup-guide-primitives.tsx` around lines 17 - 24, Add the required blank line before every return in this file: insert an empty line immediately above the early return in the status === 'completed' branch shown and do the same for the other components mentioned (StepIndicator, SetupStep, SetupButton) and the early exits in IntegrationCredentialsSidebar so each return statement is preceded by a blank line to comply with the repo rule; ensure you update the returns at the noted locations (around lines currently near 51-52, 103-104, 132, and 156) consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/agents/services/agent-credential.service.ts`:
- Around line 81-90: The current pre-validation write to
agentIntegration.connectedAt via agentIntegrationRepository.updateOne can
incorrectly mark success on transient DB failures and races; move the update to
after successful validation/decryption and perform an atomic conditional update
(filtering for { _id: agentIntegration._id, connectedAt: null, _environmentId:
environmentId, _organizationId: organizationId } and $set: { connectedAt: new
Date() }) so only the first resolver wins, and wrap the updateOne call in a
try/catch that logs errors but does not propagate them (so credential resolution
still succeeds on transient write failures). Ensure you modify the code paths
around agentIntegration.connectedAt and the callsite that currently performs the
update before validation/decryption.
In `@apps/dashboard/src/components/agents/setup-guide-primitives.tsx`:
- Around line 124-156: The component currently returns null when integration or
provider is missing, which hides loading vs not-found states; update the logic
around useFetchIntegrations() and integration/provider resolution so you handle
loading and error states instead of silently returning: use the loading/error
flags from useFetchIntegrations (e.g., isLoading/isError) and, when the sidebar
is open, render a loader while isLoading, render a clear "integration not found"
or provider error UI when !integration or !provider, or alternatively accept the
resolved integration as a prop rather than refetching here; update the
conditional that now does "if (!integration || !provider) return null" to show
loading/error UI and keep onSubmit/integrationId logic unchanged (functions:
useFetchIntegrations, integrationId, integration, provider, onSubmit).
- Around line 86-107: When href is present avoid nesting <Button /> inside an
<a>; instead render a single anchor element styled like the button. Replace the
branch that returns <a><Button/></a> by returning an <a> that contains the same
children/leadingIcon/RiArrowRightUpLine markup used in content, and move
className/size/variant styling onto the <a>. Ensure disabled behavior by not
setting the href when disabled (or set aria-disabled="true" and tabIndex={-1}
and add a pointer-events-none/opacity class) and preserve onClick only for the
non-href case; adjust the code around the content variable, Button component
usage, href, onClick, disabled and RiArrowRightUpLine accordingly.
---
Nitpick comments:
In `@apps/dashboard/src/components/agents/setup-guide-primitives.tsx`:
- Around line 17-24: Add the required blank line before every return in this
file: insert an empty line immediately above the early return in the status ===
'completed' branch shown and do the same for the other components mentioned
(StepIndicator, SetupStep, SetupButton) and the early exits in
IntegrationCredentialsSidebar so each return statement is preceded by a blank
line to comply with the repo rule; ensure you update the returns at the noted
locations (around lines currently near 51-52, 103-104, 132, and 156)
consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2c0e5a0f-1a1c-4f85-874b-d9278547a9c4
📒 Files selected for processing (7)
apps/api/src/app/agents/services/agent-credential.service.tsapps/api/src/app/agents/usecases/update-agent/update-agent.command.tsapps/api/src/app/agents/usecases/update-agent/update-agent.usecase.tsapps/dashboard/src/components/agents/agent-sidebar-widget.tsxapps/dashboard/src/components/agents/provider-dropdown.tsxapps/dashboard/src/components/agents/setup-guide-primitives.tsxapps/dashboard/src/components/agents/slack-setup-guide.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/api/src/app/agents/usecases/update-agent/update-agent.command.ts
- apps/api/src/app/agents/usecases/update-agent/update-agent.usecase.ts
- apps/dashboard/src/components/agents/agent-sidebar-widget.tsx
- apps/dashboard/src/components/agents/provider-dropdown.tsx
- apps/dashboard/src/components/agents/slack-setup-guide.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/app/agents/services/agent-inbound-handler.service.ts (1)
215-228:⚠️ Potential issue | 🟠 MajorMissing
NoBridgeUrlErrorhandling inhandleAction().The
handleAction()method callsbridgeExecutor.execute()without catchingNoBridgeUrlError, unlike thehandle()method. With the new throwing behavior inBridgeExecutorService, this will cause unhandled exceptions when actions are triggered without a configured bridge URL.Either wrap this call in the same try/catch pattern, or document why actions should fail differently than messages.
🐛 Proposed fix to add consistent error handling
const [subscriber, history] = await Promise.all([ subscriberId ? this.subscriberRepository.findBySubscriberId(config.environmentId, subscriberId) : Promise.resolve(null), this.conversationService.getHistory(config.environmentId, conversation._id), ]); - await this.bridgeExecutor.execute({ - event: AgentEventEnum.ON_ACTION, - config, - conversation, - subscriber, - history, - message: null, - platformContext: { - threadId: thread.id, - channelId: thread.channelId, - isDM: thread.isDM, - }, - action, - }); + try { + await this.bridgeExecutor.execute({ + event: AgentEventEnum.ON_ACTION, + config, + conversation, + subscriber, + history, + message: null, + platformContext: { + threadId: thread.id, + channelId: thread.channelId, + isDM: thread.isDM, + }, + action, + }); + } catch (err) { + if (err instanceof NoBridgeUrlError) { + this.logger.warn(`[agent:${agentId}] No bridge URL configured for action ${action.actionId}, skipping`); + + return; + } + + throw err; + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/services/agent-inbound-handler.service.ts` around lines 215 - 228, handleAction() currently calls bridgeExecutor.execute({ event: AgentEventEnum.ON_ACTION, ... }) without catching NoBridgeUrlError, causing unhandled exceptions; modify handleAction() to wrap the bridgeExecutor.execute(...) call in a try/catch that specifically catches NoBridgeUrlError (and reuses the same handling logic used in handle(), e.g., logging and returning/short-circuiting), so actions triggered without a bridge URL are handled consistently with message handling.
🧹 Nitpick comments (1)
apps/api/src/app/agents/services/agent-inbound-handler.service.ts (1)
13-15: Consider extracting onboarding messages to a shared constants file.The Slack-specific markdown formatting (
*bold*) is appropriate for the current use case. If more onboarding messages are added, consider extracting them to a dedicated constants module for maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/services/agent-inbound-handler.service.ts` around lines 13 - 15, The ONBOARDING_NO_BRIDGE_REPLY_MARKDOWN constant contains Slack-specific onboarding text and should be moved into a shared constants module (e.g., an onboarding/messages or agents/onboarding constants file) so future onboarding messages are centralized; extract the constant (keep the same identifier ONBOARDING_NO_BRIDGE_REPLY_MARKDOWN), export it from the new module, import and use it from agent-inbound-handler.service.ts, and update any references to pull from the shared constants instead of being defined inline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/agents/services/agent-config-resolver.service.ts`:
- Around line 93-102: The current read-then-write for setting connectedAt causes
a race and can block/ fail resolve(); change it to an atomic update with a
conditional filter and make it non-blocking: replace the two-step check+update
with a single atomic agentIntegrationRepository.updateOne call that includes
connectedAt: null in the query (or use $setOnInsert) to set connectedAt = new
Date() only when it is not already set, and do not await it inside
resolve()—instead fire-and-forget the Promise and catch/log any errors from
agentIntegrationRepository.updateOne so resolution proceeds even if the
persistence fails.
---
Outside diff comments:
In `@apps/api/src/app/agents/services/agent-inbound-handler.service.ts`:
- Around line 215-228: handleAction() currently calls bridgeExecutor.execute({
event: AgentEventEnum.ON_ACTION, ... }) without catching NoBridgeUrlError,
causing unhandled exceptions; modify handleAction() to wrap the
bridgeExecutor.execute(...) call in a try/catch that specifically catches
NoBridgeUrlError (and reuses the same handling logic used in handle(), e.g.,
logging and returning/short-circuiting), so actions triggered without a bridge
URL are handled consistently with message handling.
---
Nitpick comments:
In `@apps/api/src/app/agents/services/agent-inbound-handler.service.ts`:
- Around line 13-15: The ONBOARDING_NO_BRIDGE_REPLY_MARKDOWN constant contains
Slack-specific onboarding text and should be moved into a shared constants
module (e.g., an onboarding/messages or agents/onboarding constants file) so
future onboarding messages are centralized; extract the constant (keep the same
identifier ONBOARDING_NO_BRIDGE_REPLY_MARKDOWN), export it from the new module,
import and use it from agent-inbound-handler.service.ts, and update any
references to pull from the shared constants instead of being defined inline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1e13f552-6d67-4a6f-afed-93b6403413d1
📒 Files selected for processing (8)
apps/api/src/app/agents/services/agent-config-resolver.service.tsapps/api/src/app/agents/services/agent-inbound-handler.service.tsapps/api/src/app/agents/services/bridge-executor.service.tsapps/api/src/app/agents/services/chat-sdk.service.tsapps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.usecase.tsapps/api/src/app/agents/usecases/update-agent/update-agent.usecase.tslibs/dal/src/repositories/agent/agent.entity.tslibs/dal/src/repositories/agent/agent.schema.ts
✅ Files skipped from review due to trivial changes (2)
- apps/api/src/app/agents/services/chat-sdk.service.ts
- libs/dal/src/repositories/agent/agent.schema.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/dal/src/repositories/agent/agent.entity.ts
- apps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.usecase.ts
- apps/api/src/app/agents/usecases/update-agent/update-agent.usecase.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/dashboard/src/components/agents/setup-guide-primitives.tsx (1)
138-170:⚠️ Potential issue | 🟠 MajorHandle loading/error/not-found states explicitly instead of returning
null.Line 170 still silently returns nothing when the query hasn’t resolved (or fails), so opening the sidebar can appear broken.
Suggested patch
- const { integrations } = useFetchIntegrations(); + const { integrations, isLoading, isError } = useFetchIntegrations(); @@ - if (!integration || !provider) return null; + if (isOpen && isLoading) { + return <div className="text-text-soft p-4 text-sm">Loading integration settings…</div>; + } + + if (isOpen && (isError || !integration || !provider)) { + return <div className="text-text-soft p-4 text-sm">Could not load integration settings.</div>; + } + + if (!integration || !provider) return null;As per coding guidelines "Check for proper loading/error states."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/agents/setup-guide-primitives.tsx` around lines 138 - 170, The component silently returns null when integration or provider is missing; update the render logic around the useFetchIntegrations/useUpdateIntegration results (and the integration/provider lookups) to explicitly handle loading, error, and not-found states: check the hooks' isLoading/isFetching/isError (or equivalent) and render a loading indicator while fetching, an error message when the query fails, and a clear "integration not found" or fallback UI when the integration/provider lookup yields no match, only rendering the existing form and calling onSubmit when data is present; reference the integration and provider variables, the useFetchIntegrations/useUpdateIntegration hooks, and the onSubmit handler when adding these conditional branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/agents/usecases/update-agent/update-agent.usecase.ts`:
- Around line 12-20: The guard in update-agent.usecase.ts allows requests where
command.behavior is an empty object, producing an empty Mongo $set and a no-op
update; after you build the update document (the object passed to updateOne)
check the constructed $set's size/keys and if there are no keys throw
BadRequestException (same message or a new one like "At least one of name,
description, behavior, or active must be provided.") before calling updateOne;
ensure this check references the same update object you pass to updateOne so
empty behavior-only payloads are rejected.
In `@apps/dashboard/src/components/agents/setup-guide-primitives.tsx`:
- Around line 16-31: The StepIndicator component currently renders visual-only
markers; update StepIndicator (props: status, index) to expose the state to
assistive tech by adding an accessible label or screen-reader text: for the
completed branch include an aria-label (or visually-hidden <span>) that reads
e.g. "Step {index} completed", and for the non-completed branch include "Step
{index} {status}" (or "incomplete"/"current" as appropriate); ensure the wrapper
element has an appropriate role/aria-live if you want dynamic announcements
(e.g., role="status" or aria-live="polite") so screen readers announce changes.
---
Duplicate comments:
In `@apps/dashboard/src/components/agents/setup-guide-primitives.tsx`:
- Around line 138-170: The component silently returns null when integration or
provider is missing; update the render logic around the
useFetchIntegrations/useUpdateIntegration results (and the integration/provider
lookups) to explicitly handle loading, error, and not-found states: check the
hooks' isLoading/isFetching/isError (or equivalent) and render a loading
indicator while fetching, an error message when the query fails, and a clear
"integration not found" or fallback UI when the integration/provider lookup
yields no match, only rendering the existing form and calling onSubmit when data
is present; reference the integration and provider variables, the
useFetchIntegrations/useUpdateIntegration hooks, and the onSubmit handler when
adding these conditional branches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3a2739b5-ee5d-4e3e-bc52-9f2ff2407459
📒 Files selected for processing (3)
apps/api/src/app/agents/services/agent-config-resolver.service.tsapps/api/src/app/agents/usecases/update-agent/update-agent.usecase.tsapps/dashboard/src/components/agents/setup-guide-primitives.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/src/app/agents/services/agent-config-resolver.service.ts
This was referenced Apr 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed? Why was the change needed?
What changed
This PR adds a Slack quickstart flow for agents: UI components and API/schema support to guide workspace installation, detect when a Slack integration is connected, and allow toggling agent active state. It also changes routing to target specific integration identifiers and adds error-handling to recover when a delivery bridge URL is missing.
Affected areas
api: Added agent
active: boolean(default true) andconnectedAton agent-integration links; updated DTOs, mappers, commands, and usecases to persist/serialize these fields; addedNoBridgeUrlErrorand improved bridge executor/inbound handler error flow to send an onboarding reply when no bridge URL exists.dashboard: New quickstart UI —
AgentOverviewTab,AgentSidebarWidget,AgentSetupGuide,SlackSetupGuide,ProviderDropdown, and setup primitives; switched integrations routing/props fromproviderId→integrationIdentifier; added agent PATCH client, updated types to includeactiveandconnectedAt, and adjusted integration creation payload typing.dal: Persisted
activeon Agent schema/entity with default true and added optionalconnectedAtDate|null on AgentIntegration schema/entity.shared: Added
conversational-providersmetadata (CONVERSATIONAL_PROVIDERSand IDs) to list conversational provider options (Slack enabled, others marked coming soon).Key technical decisions
connectedAtto track first successful integration connection, enabling polling-based workspace detection in the UI.providerIdwithintegrationIdentifierto scope guides to a specific integration instance.NoBridgeUrlErrorto distinguish bridge-missing cases and trigger a user-facing onboarding reply.Testing
No automated tests were added; manual verification is required for the Slack setup flow, the polling-based connection detection, agent update (activate/deactivate) behavior, and the new bridge error recovery path.
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer