Add Tier 1-3 enhancements: webhooks, segments, reports, and more #338
Conversation
Tier 1 (Quick Wins): - Add email resend functionality - Add domain reputation badges - Improve email search with filters - Add campaign comparison page - Improve template preview Tier 2 (Medium Effort): - Add webhook system with delivery tracking - Add bulk suppression management - Add contact duplicate detection - Add campaign comparison analytics Tier 3 (Higher Effort): - Add API usage dashboard with daily metrics - Add contact segmentation with filter builder - Add scheduled reports with daily/weekly/monthly options - Create alert-dialog UI component Technical changes: - Add ScheduledReport, Segment, Webhook, WebhookDelivery models - Add segment router with dynamic filter building - Add scheduled-report router with next-send-date calculation - Add webhook router and delivery service - Enhance email router with search and resend - Add @radix-ui/react-alert-dialog dependency Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Someone is attempting to deploy a commit to the kmkoushik's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Rate limit exceeded@mohamed-elkholy95 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 28 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughAdds multiple backend routers, Prisma models, and services plus extensive frontend UI for webhooks, A/B tests, segments, scheduled reports, sequences, send-time optimization, audit logs, duplicate contact management, campaign comparison, and API usage analytics. Introduces webhook dispatch/queueing, HTML sanitization utility, resend email workflow, date-range email filters, domain-aware mailer selection, and several new UI components/pages for managing the above. Expands the public TRPC API surface and updates package dependencies. Possibly related PRs
Suggested labels
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
27 issues found across 62 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/src/server/service/email-service.ts">
<violation number="1" location="apps/web/src/server/service/email-service.ts:382">
P1: Allowing resend of "COMPLAINED" status emails risks violating anti-spam regulations and damaging sender reputation. Recipients who marked emails as spam should not receive resent emails.</violation>
<violation number="2" location="apps/web/src/server/service/email-service.ts:416">
P1: The resendEmail function doesn't copy attachments and headers from the original email. Resent emails will lose these fields, causing incomplete email delivery.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/dev-settings/webhooks/[webhookId]/page.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/dev-settings/webhooks/[webhookId]/page.tsx:140">
P2: Add error handling for clipboard API. The navigator.clipboard.writeText() can fail due to permissions, browser support, or non-secure contexts.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/contacts/[contactBookId]/page.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/contacts/[contactBookId]/page.tsx:125">
P1: Avoid nesting Button inside Link. Use `asChild` prop on Button to properly compose with Link and prevent invalid HTML (<a><button>).</violation>
</file>
<file name="apps/web/src/app/(dashboard)/contacts/[contactBookId]/segments/[segmentId]/page.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/contacts/[contactBookId]/segments/[segmentId]/page.tsx:26">
P2: Pagination state doesn't reset when segmentId changes, causing empty results when navigating between segments. Add useEffect to reset page to 1 when segmentId changes.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsx:278">
P0: This will crash during server-side rendering because DOMPurify requires the browser `window` object. The sanitize-html.ts utility imports DOMPurify at the top level without SSR protection, causing "window is not defined" errors when Next.js performs initial SSR (even for "use client" components). Either use `isomorphic-dompurify` package or add a server-side check in sanitize-html.ts: `if (typeof window === 'undefined') return html || "";`</violation>
</file>
<file name="packages/email-editor/src/renderer.tsx">
<violation number="1" location="packages/email-editor/src/renderer.tsx:561">
P2: Inconsistent alignment attribute migration. The `image` method should be updated to support both `textAlign` (preferred) and `alignment` (legacy) attributes, matching the pattern used in the `button` and `logo` methods to ensure a consistent API across all alignment-supporting components.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/contacts/page.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/contacts/page.tsx:16">
P1: Avoid nesting Button inside Link. Use the `asChild` prop pattern instead: `<Button asChild variant="outline"><Link href="/contacts/duplicates">...</Link></Button>`. This prevents nested interactive elements and accessibility issues.</violation>
</file>
<file name="apps/web/src/server/api/routers/webhook.ts">
<violation number="1" location="apps/web/src/server/api/routers/webhook.ts:45">
P0: Using `findUnique` with both `id` and `teamId` will fail at runtime. Change to `findFirst` to filter by multiple fields.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/contacts/duplicates/duplicates-list.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/contacts/duplicates/duplicates-list.tsx:106">
P2: Pagination display is misleading. Shows current page item count as if it's total items shown. Should display the actual range (e.g., 'Showing 21-40 of 100') or current page number.</violation>
</file>
<file name="apps/web/src/server/api/routers/campaign.ts">
<violation number="1" location="apps/web/src/server/api/routers/campaign.ts:316">
P2: Missing validation that all requested campaigns exist. The input requires 2-5 campaign IDs, but the query may return fewer if campaigns don't exist or belong to another team, potentially causing unexpected behavior on the frontend.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/settings/reports/delete-report.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/settings/reports/delete-report.tsx:53">
P2: Disable the delete button while the mutation is pending to prevent multiple rapid clicks and improve UX.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/settings/reports/reports-list.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/settings/reports/reports-list.tsx:36">
P1: The timezone parameter is received but never used when formatting the schedule time. This causes the time to display in the browser's current timezone instead of the scheduled timezone, leading to incorrect time display for users in different timezones.</violation>
</file>
<file name="apps/web/prisma/schema.prisma">
<violation number="1" location="apps/web/prisma/schema.prisma:367">
P2: Add validation constraints for hour (0-23), dayOfWeek (0-6), and dayOfMonth (1-31) fields. Consider adding application-level validation or custom migration with CHECK constraints to prevent invalid scheduling values.</violation>
<violation number="2" location="apps/web/prisma/schema.prisma:507">
P2: Consider encrypting webhook secrets at rest. While webhook secrets need to be retrievable (unlike passwords), storing them in plaintext increases risk if the database is compromised.</violation>
<violation number="3" location="apps/web/prisma/schema.prisma:529">
P1: Add foreign key relation for emailId to maintain referential integrity with the Email model. Without this, WebhookDelivery can reference non-existent emails.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/domains/domain-list.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/domains/domain-list.tsx:86">
P1: N+1 query problem: Each domain renders DomainReputationBadge which makes a separate API call. With many domains, this causes excessive API requests and performance degradation. Consider fetching all reputation metrics in a single batched query at the parent level or implementing request batching.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/dev-settings/usage/page.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/dev-settings/usage/page.tsx:154">
P1: Performance issue: `maxTotal` is recalculated inside the loop on every iteration. For 90 days of data, this creates O(N²) complexity. Calculate it once before the map.</violation>
</file>
<file name="apps/web/src/server/api/routers/scheduled-report.ts">
<violation number="1" location="apps/web/src/server/api/routers/scheduled-report.ts:29">
P0: Timezone parameter is stored but never used in date calculations. Reports will fire at the wrong time for users in different timezones. Use a timezone-aware library like `date-fns-tz` or convert dates to the specified timezone before scheduling.</violation>
<violation number="2" location="apps/web/src/server/api/routers/scheduled-report.ts:105">
P1: Missing validation: WEEKLY frequency requires dayOfWeek, MONTHLY requires dayOfMonth. Add conditional validation using Zod's refine to ensure required fields are present for each frequency type.</violation>
<violation number="3" location="apps/web/src/server/api/routers/scheduled-report.ts:172">
P1: Recalculation logic fails when hour is set to 0 (midnight) because 0 is falsy. Use `input.hour !== undefined` instead of just `input.hour` to properly detect when the field is provided.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/domains/domain-reputation-badge.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/domains/domain-reputation-badge.tsx:44">
P2: Missing error handling for tRPC query. Add `isError` or `error` check to distinguish between "no data available" and "query failed" states.</violation>
<violation number="2" location="apps/web/src/app/(dashboard)/domains/domain-reputation-badge.tsx:83">
P2: Tooltip trigger uses non-focusable div, making it inaccessible to keyboard users. Change to a button element or add tabIndex={0} and onKeyDown handler for keyboard accessibility.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/contacts/[contactBookId]/segments/delete-segment.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/contacts/[contactBookId]/segments/delete-segment.tsx:57">
P1: Add `disabled={deleteMutation.isPending}` to prevent multiple submissions while the delete operation is in progress.</violation>
</file>
<file name="apps/web/src/server/service/webhook-service.ts">
<violation number="1" location="apps/web/src/server/service/webhook-service.ts:65">
P2: Static Worker and Queue instances are never closed, leading to resource leaks on shutdown. Consider adding a shutdown method that calls worker.close() and queue.close() to enable graceful shutdown.</violation>
<violation number="2" location="apps/web/src/server/service/webhook-service.ts:194">
P1: Timeout cleared before reading response body, leaving response.text() unprotected. Move clearTimeout() after reading the response body to ensure the entire operation is protected by the timeout.</violation>
<violation number="3" location="apps/web/src/server/service/webhook-service.ts:199">
P2: Database updates lack error handling. If the database update fails after a successful webhook delivery, the success won't be recorded. Wrap database operations in try-catch to handle failures gracefully and log appropriately.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| dayOfWeek?: number | null, | ||
| dayOfMonth?: number | null, | ||
| ): Date { | ||
| let nextDate = new Date(); |
There was a problem hiding this comment.
P0: Timezone parameter is stored but never used in date calculations. Reports will fire at the wrong time for users in different timezones. Use a timezone-aware library like date-fns-tz or convert dates to the specified timezone before scheduling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/server/api/routers/scheduled-report.ts, line 29:
<comment>Timezone parameter is stored but never used in date calculations. Reports will fire at the wrong time for users in different timezones. Use a timezone-aware library like `date-fns-tz` or convert dates to the specified timezone before scheduling.</comment>
<file context>
@@ -0,0 +1,235 @@
+ dayOfWeek?: number | null,
+ dayOfMonth?: number | null,
+): Date {
+ let nextDate = new Date();
+
+ // Set the time
</file context>
| frequency ReportFrequency | ||
| dayOfWeek Int? // 0-6 for weekly reports (0 = Sunday) | ||
| dayOfMonth Int? // 1-31 for monthly reports | ||
| hour Int @default(9) // Hour to send (0-23) |
There was a problem hiding this comment.
P2: Add validation constraints for hour (0-23), dayOfWeek (0-6), and dayOfMonth (1-31) fields. Consider adding application-level validation or custom migration with CHECK constraints to prevent invalid scheduling values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/prisma/schema.prisma, line 367:
<comment>Add validation constraints for hour (0-23), dayOfWeek (0-6), and dayOfMonth (1-31) fields. Consider adding application-level validation or custom migration with CHECK constraints to prevent invalid scheduling values.</comment>
<file context>
@@ -333,6 +336,48 @@ model Contact {
+ frequency ReportFrequency
+ dayOfWeek Int? // 0-6 for weekly reports (0 = Sunday)
+ dayOfMonth Int? // 1-31 for monthly reports
+ hour Int @default(9) // Hour to send (0-23)
+ timezone String @default("UTC")
+ enabled Boolean @default(true)
</file context>
| <TooltipProvider> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <div |
There was a problem hiding this comment.
P2: Tooltip trigger uses non-focusable div, making it inaccessible to keyboard users. Change to a button element or add tabIndex={0} and onKeyDown handler for keyboard accessibility.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/app/(dashboard)/domains/domain-reputation-badge.tsx, line 83:
<comment>Tooltip trigger uses non-focusable div, making it inaccessible to keyboard users. Change to a button element or add tabIndex={0} and onKeyDown handler for keyboard accessibility.</comment>
<file context>
@@ -0,0 +1,110 @@
+ <TooltipProvider>
+ <Tooltip>
+ <TooltipTrigger asChild>
+ <div
+ className={`inline-flex items-center gap-1 px-2 py-1 rounded-md text-xs border ${className}`}
+ >
</file context>
| } | ||
|
|
||
| export function DomainReputationBadge({ domainId }: { domainId: number }) { | ||
| const { data: metrics, isLoading } = |
There was a problem hiding this comment.
P2: Missing error handling for tRPC query. Add isError or error check to distinguish between "no data available" and "query failed" states.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/app/(dashboard)/domains/domain-reputation-badge.tsx, line 44:
<comment>Missing error handling for tRPC query. Add `isError` or `error` check to distinguish between "no data available" and "query failed" states.</comment>
<file context>
@@ -0,0 +1,110 @@
+}
+
+export function DomainReputationBadge({ domainId }: { domainId: number }) {
+ const { data: metrics, isLoading } =
+ api.dashboard.reputationMetricsData.useQuery({
+ domain: domainId,
</file context>
| connection: getRedis(), | ||
| }); | ||
|
|
||
| private static worker = new Worker<WebhookDeliveryJob>( |
There was a problem hiding this comment.
P2: Static Worker and Queue instances are never closed, leading to resource leaks on shutdown. Consider adding a shutdown method that calls worker.close() and queue.close() to enable graceful shutdown.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/server/service/webhook-service.ts, line 65:
<comment>Static Worker and Queue instances are never closed, leading to resource leaks on shutdown. Consider adding a shutdown method that calls worker.close() and queue.close() to enable graceful shutdown.</comment>
<file context>
@@ -0,0 +1,299 @@
+ connection: getRedis(),
+ });
+
+ private static worker = new Worker<WebhookDeliveryJob>(
+ WEBHOOK_DELIVERY_QUEUE,
+ async (job) => {
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
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)
packages/ui/src/form.tsx (1)
42-63: Also guardFormItemContext(and/or fix the error message) to avoidundefined-*IDs.
useFormField()throws for missing<FormField>, but it can still run outside<FormItem>and produceid === undefined-derived attributes. Consider checking both contexts and updating the message accordingly.Proposed patch
const useFormField = () => { const fieldContext = React.useContext(FormFieldContext); const itemContext = React.useContext(FormItemContext); const { getFieldState, formState } = useFormContext(); - if (!fieldContext?.name) { - throw new Error("useFormField should be used within <FormField>"); + if (!fieldContext?.name || !itemContext?.id) { + throw new Error("useFormField should be used within <FormField> and <FormItem>"); } const fieldState = getFieldState(fieldContext.name, formState); const { id } = itemContext;packages/email-editor/src/renderer.tsx (2)
652-716: Inconsistency: Image method not updated per AI summary.The AI summary indicates that
textAlignsupport was added to button, logo, and image renderers. However, theimagemethod shows no changes and continues to use only thealignmentattribute (line 660, 696) without supporting the preferredtextAlignattribute.This creates an inconsistent API where button and logo support
textAlignbut image does not.♻️ Proposed update to align with button/logo pattern
private image(node: JSONContent, options?: NodeOptions): React.ReactNode { const { attrs } = node; const { src, alt, title, width = "auto", height = "auto", - alignment = "center", + textAlign, + alignment, externalLink = "", borderRadius, borderColor, borderWidth, } = attrs || {}; + // Support both textAlign (preferred) and alignment (legacy) attributes + const imageAlignment = textAlign ?? alignment ?? "center"; const { next } = options || {}; const isNextSpacer = next?.type === "spacer"; const mainImage = ( <Img alt={alt || title || "Image"} src={src} style={{ height, width, maxWidth: "100%", outline: "none", textDecoration: "none", borderStyle: "solid", borderRadius: `${borderRadius}px`, borderColor, borderWidth: `${borderWidth}px`, }} title={title || alt || "Image"} /> ); return ( <Row style={{ marginTop: "0px", marginBottom: isNextSpacer ? "0px" : "32px", }} > - <Column align={alignment}> + <Column align={imageAlignment}> {externalLink ? ( <a href={externalLink} rel="noopener noreferrer" style={{ display: "block", maxWidth: "100%", textDecoration: "none", }} target="_blank" > {mainImage} </a> ) : ( mainImage )} </Column> </Row> ); }
13-13: Fix incorrect Button import from jsx-email.The import
Butan as Buttonis a typo. The correct named export fromjsx-emailv2.7.1 isButton. This import will fail at runtime sinceButandoes not exist as an export.🔧 Fix
- Butan as Button, + Button,
🤖 Fix all issues with AI agents
In @apps/web/src/app/(dashboard)/campaigns/compare/comparison-view.tsx:
- Around line 59-67: The code assumes non-empty and fully populated campaign
data which can crash: guard the campaigns array and nullable rate fields before
using Math.max, toFixed or computing widths. In the maxValues object (delivered,
openRate, clickRate, deliveryRate) replace Math.max(...campaigns.map(...)) with
Math.max(0, ...campaigns.map(c => Number(c?.openRate ?? c?.openRate) || 0)) (or
similar) so spreading an empty array yields 0 and each field coerces to a safe
number; similarly, when calling toFixed or computing widths later (the places
referenced around the other comparison rendering code), use Number(value ?? 0)
or (value ?? 0).toFixed(...) and guard divisions by zero to avoid NaN widths.
Ensure all accesses to c.delivered, c.openRate, c.clickRate, c.deliveryRate use
nullish coalescing or Number(...) || 0 so the UI never receives undefined/NaN.
In @apps/web/src/app/(dashboard)/contacts/page.tsx:
- Line 6: The Button import in page.tsx uses a deep path
("@usesend/ui/src/button") which is inconsistent with other imports (e.g., H1)
from "@usesend/ui"; update the import for Button to import from the package root
("@usesend/ui") to match the other imports and avoid bundling/consumption
issues, ensuring you only change the import statement that references Button and
leave the rest of the file intact.
In @apps/web/src/app/(dashboard)/dev-settings/webhooks/add-webhook.tsx:
- Around line 65-83: handleSave currently bypasses type safety by using "as any"
for events and invalidates all webhook queries; change it to pass a properly
typed events array (derive the type from webhookSchema or a shared WebhookEvent
type and validate/parse values.events instead of casting) when calling
createMutation.mutate, and scope cache invalidation to only the list view by
replacing utils.webhook.invalidate() with the list-specific invalidation helper
(e.g., utils.webhook.list.invalidate()) so only the webhook list is refreshed;
keep setSecret and form.reset behavior unchanged.
In @apps/web/src/app/(dashboard)/domains/domain-reputation-badge.tsx:
- Around line 43-61: The DomainReputationBadge component lacks handling for
query errors from api.dashboard.reputationMetricsData.useQuery; update the hook
call to destructure error and/or isError (e.g., { data: metrics, isLoading,
isError, error }) and add a branch that returns a small error UI when isError
(or error is truthy) — for example a compact "Error" or icon with text and
optional tooltip containing error.message — so the component doesn't hang or
crash on failed requests.
In @apps/web/src/app/(dashboard)/settings/reports/create-report.tsx:
- Around line 52-60: createReportSchema currently marks dayOfWeek and dayOfMonth
optional but must enforce them when frequency is WEEKLY or MONTHLY; update
createReportSchema to add a conditional validation (using .superRefine or
refinements) that errors if frequency === "WEEKLY" and dayOfWeek is
missing/empty, or if frequency === "MONTHLY" and dayOfMonth is missing/empty,
and validate their formats if needed; then remove the fallback operators (the
`?? "1"` usages) where the form values are read/submitted so missing values fail
schema validation instead of being masked.
In @apps/web/src/app/(dashboard)/settings/reports/reports-list.tsx:
- Around line 11-13: The Spinner is imported as a default export but the module
exports it as a named export; change the import to a named import: import {
Spinner } from "@usesend/ui/src/spinner"; and update any other occurrences in
this file that import Spinner as default (also the other Spinner imports later
in this file) so all usages that render <Spinner /> match the named import.
In @apps/web/src/server/api/routers/contacts.ts:
- Around line 261-294: The mergeDuplicates mutation isn’t atomic, deletes
without re-checking team/email, and throws a generic Error; wrap the
verification + delete in a single DB transaction (use db.$transaction or
equivalent) so both the find and delete happen atomically, constrain the
deleteMany query to include the same contactBook.teamId and email filters (so
db.contact.deleteMany filters by id in deleteContactIds AND contactBook.teamId
=== team.id AND email === input.email), and replace the thrown Error with a
TRPCError (e.g., new TRPCError({ code: "NOT_FOUND", message: "Some contacts not
found or do not belong to your team" })) to return the proper client error.
In @apps/web/src/server/api/routers/scheduled-report.ts:
- Around line 163-175: The conditional that decides whether to recalculate
nextSendAt uses truthiness checks so setting input.hour = 0 is treated as falsy
and skips recomputation; change the condition to explicitly test for undefined
(e.g., input.frequency !== undefined || input.hour !== undefined ||
input.dayOfWeek !== undefined || input.dayOfMonth !== undefined) so that any
provided value including 0 triggers calculateNextSendAt(frequency, hour,
dayOfWeek, dayOfMonth) using the existing/nullish-coalesced values.
- Around line 99-110: The API accepts dayOfMonth up to 31 and
calculateNextSendAt uses setDate(dayOfMonth) which overflows months; change the
Zod schema in the create procedure (the dayOfMonth validator) to
z.number().min(1).max(28).optional() and also update calculateNextSendAt to
clamp any provided dayOfMonth via Math.min(dayOfMonth, 28) (or equivalent)
before calling setDate so behavior matches the UI promise of falling back to the
28th on shorter months.
In @apps/web/src/server/api/routers/segment.ts:
- Around line 73-84: The createdAt handling block (where field === "createdAt")
currently uses new Date(String(value)) without validating for Invalid Date;
update it to validate dateValue with !isNaN(dateValue.getTime()) before
returning Prisma filters (e.g., when operator === "greater_than" return {
createdAt: { gte: dateValue } } only if the check passes), and otherwise return
null (or surface a validation error), or alternatively use z.coerce.date() at
the input schema level to ensure createdAt is always a valid Date before this
code runs.
- Around line 10-36: The filterConditionSchema currently allows invalid
field/operator combos and unvalidated date strings; update it to a discriminated
union keyed by field (i.e., replace the generic filterConditionSchema with
per-field schemas for "email", "firstName", "lastName", "subscribed",
"createdAt", "property") that explicitly list only the allowed operators for
each field and mark non-nullable fields (e.g., email) as rejecting
is_empty/is_not_empty, ensure "subscribed" only accepts boolean-relevant
operators (is_true/is_false, equals/not_equals with boolean), and make
"createdAt" accept only date-comparison operators while validating/parsing the
date string to a real Date (reject on invalid date) before it can be used;
implement these checks in the Zod schema so invalid combos throw Zod errors
instead of returning null.
- Around line 86-99: The JSON path used when handling the "property" field is
incorrect: change the path construction in the segment filter block (where field
=== "property" && propertyKey) from including the field name to being relative
to the JSON field; replace the current path value that uses ["properties",
propertyKey] with a single-element array containing propertyKey so the Prisma
JSON filter on Contact.properties uses [propertyKey] instead of ["properties",
propertyKey].
In @apps/web/src/server/api/routers/webhook.ts:
- Around line 8-18: Replace the hardcoded Zod enum with a native Prisma enum:
import the Prisma enum (WebhookEventType) from @prisma/client and change
webhookEventTypes from z.array(z.enum([...])) to
z.array(z.nativeEnum(WebhookEventType)); then remove the manual type assertions
where this schema is used (the locations currently casting to
WebhookEventType[]), since z.nativeEnum will correctly infer the Prisma enum
type.
In @apps/web/src/server/service/email-service.ts:
- Around line 415-432: resendEmail currently omits attachments and headers when
creating the new email record; update the data passed to db.email.create inside
resendEmail (the block creating newEmail) to include the original email's
attachments and headers (e.g., use originalEmail.attachments and
originalEmail.headers), ensuring you handle null/undefined (default to empty
array/object) and match the DB/Prisma field shape so attachments and custom
headers are preserved on the resent message.
In @apps/web/src/server/service/webhook-service.ts:
- Around line 61-74: The static initialization of webhookQueue and worker
(WEBHOOK_DELIVERY_QUEUE using getRedis()) can crash module load if Redis is
unavailable; change to lazy initialization and resilient startup by replacing
the static webhookQueue and worker with getter functions or init methods that
create Queue<WebhookDeliveryJob> and Worker<WebhookDeliveryJob> on first use (or
explicitly call init at app start), wrap getRedis() calls and new Worker(...)
creation in try/catch with retries/backoff, and ensure the worker's processor
(which calls this.deliverWebhook) is registered only after a successful
connection; reference webhookQueue, worker, WEBHOOK_DELIVERY_QUEUE, getRedis(),
deliverWebhook, and WebhookDeliveryJob when locating and updating the code.
In @apps/web/src/utils/sanitize-html.ts:
- Around line 1-2: This file imports DOMPurify which requires a browser DOM and
will break during SSR; add the "use client" directive as the very first line of
apps/web/src/utils/sanitize-html.ts so the module (and the DOMPurify import) is
only evaluated on the client, then keep the existing import of DOMPurify and any
sanitize function (e.g., sanitizeHtml) unchanged.
🟡 Minor comments (16)
apps/web/src/app/(dashboard)/contacts/duplicates/merge-duplicates.tsx-57-66 (1)
57-66: Dialog description shows stale count before data loads.When
detailsQuery.isLoadingis true,contacts.lengthwill be0(from the default fallback), causing the description to incorrectly state "This email appears in 0 contact books." Consider conditionally rendering the count or moving the description inside the loaded state.Proposed fix
<DialogHeader> <DialogTitle>Manage Duplicate: {email}</DialogTitle> <DialogDescription> - This email appears in {contacts.length} contact books. Select which - contact to keep and the others will be deleted. + {detailsQuery.isLoading + ? "Loading duplicate contacts..." + : `This email appears in ${contacts.length} contact books. Select which contact to keep and the others will be deleted.`} </DialogDescription> </DialogHeader>apps/web/src/app/(dashboard)/contacts/duplicates/duplicates-list.tsx-33-51 (1)
33-51: Missing error state handling for the duplicates query.If
duplicatesQueryfails, users see the empty state instead of an error message. This could be confusing since the empty state implies success with no duplicates.Proposed fix
if (duplicatesQuery.isLoading) { return ( <div className="flex justify-center items-center h-64"> <Spinner className="w-6 h-6" /> </div> ); } + if (duplicatesQuery.isError) { + return ( + <div className="text-center py-16 border rounded-xl bg-destructive/10"> + <p className="text-destructive">Failed to load duplicates. Please try again.</p> + </div> + ); + } + if (duplicates.length === 0) {apps/web/src/app/(dashboard)/contacts/duplicates/merge-duplicates.tsx-68-129 (1)
68-129: Missing error state handling for the details query.If
detailsQueryfails, users see an empty radio group with no feedback. Consider adding error state handling to inform users when data fetching fails.Proposed fix
{detailsQuery.isLoading ? ( <div className="flex justify-center py-8"> <Spinner className="w-6 h-6" /> </div> + ) : detailsQuery.isError ? ( + <div className="text-center py-8 text-destructive"> + Failed to load duplicate details. Please try again. + </div> ) : ( <div className="space-y-4 max-h-[400px] overflow-y-auto">apps/web/package.json-46-46 (1)
46-46: Update dompurify to the latest stable version 3.3.1.Version 3.2.4 is secure and fixed the CVE-2025-26791 vulnerability, but it is not the latest stable release. Version 3.3.1 (released Dec 8, 2025) is available. Consider updating to stay current with the latest stable release. Additionally, update
@types/dompurifyfrom 3.2.0 to align with the main package version.apps/web/src/server/api/routers/contacts.ts-165-231 (1)
165-231: Use.int()on pagination inputs, group email case-insensitively, and fix array pairing structure.
pageandlimitshould bez.number().int().min(1)to prevent float offsets; the pattern is established inadmin.tsandcampaign-schema.ts.- Emails are normalized across the codebase (
suppression-service.ts,gravatar-utils.ts); useGROUP BY LOWER(c.email)and return the canonical lowercase email to catchTest@x.comvstest@x.com.contactBookIdsandcontactBookNamesas separate parallel arrays risk id↔name pairing loss. Usearray_agg(jsonb_build_object('id', c."contactBookId", 'name', cb.name))to keep the relationship intact.- In
mergeDuplicates(line 287–291), thedeleteManyshould include a team validation constraint (e.g., nestedcontactBook.teamIdin the where clause) to prevent TOCTOU; currently only the initialfindManyvalidates team ownership.apps/web/src/server/api/routers/settings.ts-5-10 (1)
5-10: Add role-based access control to SMTP settings endpoint.The
getSmtpSettingsendpoint usesteamProcedure, which allows any authenticated team member to access SMTP configuration. Consider restricting this to admin-only viateamAdminProcedureto align with other sensitive settings like API key management. WhileSMTP_USERis a simple username (e.g.,"usesend") rather than an email address, andSMTP_HOSTdefaults to"smtp.usesend.com", limiting visibility to admins reduces unnecessary exposure of infrastructure configuration.apps/web/src/app/(dashboard)/dev-settings/webhooks/webhook-list.tsx-38-52 (1)
38-52: Improve error handling in the test handler.The catch block on line 47 ignores the error parameter, preventing specific error details from being shown to the user.
🔧 Proposed fix
- } catch { - toast.error("Failed to test webhook"); + } catch (error) { + toast.error(error instanceof Error ? error.message : "Failed to test webhook"); } finally { setTestingId(null); }apps/web/src/app/(dashboard)/dev-settings/webhooks/add-webhook.tsx-85-89 (1)
85-89: Add error handling for clipboard operations.Line 86 calls
navigator.clipboard.writeTextwithout error handling. The clipboard API can fail if the user denies permission or if the browser doesn't support it.🔧 Proposed fix
- function handleCopy() { - navigator.clipboard.writeText(secret); + async function handleCopy() { + try { + await navigator.clipboard.writeText(secret); + } catch { + toast.error("Failed to copy to clipboard"); + return; + } setIsCopied(true); setTimeout(() => setIsCopied(false), 2000); }Also update
copyAndCloseto be async and awaithandleCopy().apps/web/src/app/(dashboard)/dev-settings/smtp/page.tsx-22-28 (1)
22-28: Avoid “Create an API key” prompt while API keys query is still loading (or errored).
hasApiKeysis false during loading, so the UI can incorrectly tell users to create a key.Proposed change
const smtpQuery = api.settings.getSmtpSettings.useQuery(); const apiKeysQuery = api.apiKey.apiKeys.useQuery(); @@ - const hasApiKeys = apiKeysQuery.data && apiKeysQuery.data.length > 0; + const hasApiKeys = (apiKeysQuery.data?.length ?? 0) > 0; + const showMissingKeysHint = apiKeysQuery.isFetched && !hasApiKeys; @@ - {!hasApiKeys && ( + {showMissingKeysHint && ( <p className="mt-2 text-sm text-muted-foreground">Also applies to: 89-99
apps/web/src/app/(dashboard)/contacts/[contactBookId]/segments/create-segment.tsx-133-152 (1)
133-152: Block creating/previewing filters that “need a value” but have an empty value.Right now
contains ""will likely match everything (surprising “Preview” + persisted segment).Proposed change (client-side validation)
const handleSubmit = () => { @@ if (filters.length === 0) { toast.error("Please add at least one filter"); return; } + if ( + filters.some( + (f) => needsValue(f.operator) && String(f.value ?? "").trim().length === 0, + ) + ) { + toast.error("Please fill in all filter values"); + return; + } createMutation.mutate({ contactBookId, name, description: description || undefined, filters, }); };Also applies to: 236-244
apps/web/src/app/(dashboard)/settings/reports/reports-list.tsx-54-74 (1)
54-74: Prevent rapid toggle spam while the mutation is pending (avoids flip-flop UX).Even with optimistic updates, multiple quick toggles can feel broken if the final server state “snaps back” on settle.
Proposed change
<Switch checked={report.enabled} + disabled={toggleMutation.isPending} onCheckedChange={() => toggleMutation.mutate({ id: report.id }) } />Also applies to: 136-141
apps/web/src/app/(dashboard)/dev-settings/webhooks/[webhookId]/page.tsx-50-61 (1)
50-61: Log caught errors for debugging.The empty catch block at lines 58-60 silently swallows the error without logging it, making debugging difficult when webhook tests fail for unexpected reasons.
🔍 Proposed fix to log errors
} catch { + console.error("Failed to test webhook:", error); toast.error("Failed to test webhook"); }apps/web/src/app/(dashboard)/contacts/[contactBookId]/segments/[segmentId]/page.tsx-57-61 (1)
57-61: Validate filter structure before type assertion.The type assertion assumes filters conform to the expected shape without validation. If the stored JSON has a different structure, this could cause runtime errors when accessing
filter.field,filter.operator, orfilter.value.✅ Proposed fix with runtime validation
- const filters = segment.filters as Array<{ - field: string; - operator: string; - value?: string; - }>; + const filters = Array.isArray(segment.filters) + ? (segment.filters as Array<{ + field: string; + operator: string; + value?: string; + }>) + : [];apps/web/src/app/(dashboard)/dev-settings/webhooks/[webhookId]/page.tsx-63-72 (1)
63-72: Log caught errors for debugging.Similar to the test handler, the empty catch block at lines 69-71 loses error context that would be valuable for troubleshooting secret regeneration failures.
🔍 Proposed fix to log errors
} catch { + console.error("Failed to regenerate secret:", error); toast.error("Failed to regenerate secret"); }Committable suggestion skipped: line range outside the PR's diff.
apps/web/src/app/(dashboard)/dev-settings/webhooks/[webhookId]/page.tsx-139-142 (1)
139-142: Handle clipboard API failures.
navigator.clipboard.writeText()can fail in insecure contexts (non-HTTPS) or when clipboard permissions are denied. The current implementation assumes success.🛡️ Proposed fix with error handling
onClick={() => { - navigator.clipboard.writeText(newSecret); - toast.success("Copied to clipboard"); + navigator.clipboard.writeText(newSecret) + .then(() => toast.success("Copied to clipboard")) + .catch(() => toast.error("Failed to copy to clipboard")); }}apps/web/src/app/(dashboard)/contacts/[contactBookId]/segments/page.tsx-82-82 (1)
82-82: Validate filter structure before type assertion.The type assertion assumes filters is an array with objects containing a
fieldproperty. If the stored JSON has a different structure, this could cause runtime errors when accessingfilter.field.✅ Proposed fix with runtime validation
- const filters = segment.filters as Array<{ field: string }>; + const filters = Array.isArray(segment.filters) + ? (segment.filters as Array<{ field: string }>) + : [];
🧹 Nitpick comments (27)
apps/web/src/app/(dashboard)/contacts/page.tsx (1)
15-23: Consider a more semantically appropriate icon.The implementation follows the established pattern from the campaigns page and works correctly. However, the
Copyicon is typically associated with "copy to clipboard" actions. For a "Find Duplicates" feature, consider icons likeUsers,UserCheck, orMergefrom lucide-react for better semantic clarity.💡 Alternative icon suggestions
-import { Copy } from "lucide-react"; +import { Users } from "lucide-react"; <Link href="/contacts/duplicates"> <Button variant="outline"> - <Copy className="h-4 w-4 mr-2" /> + <Users className="h-4 w-4 mr-2" /> Find Duplicates </Button> </Link>Other options:
UserCheck,Merge, orSearchCheckdepending on your preferred visual metaphor.apps/web/src/app/(dashboard)/contacts/duplicates/page.tsx (1)
13-17: Add accessible label to the back button.The icon-only back button lacks an accessible name for screen reader users. Consider adding
aria-labelor visually hidden text.Proposed fix
<Link href="/contacts"> - <Button variant="ghost" size="sm"> + <Button variant="ghost" size="sm" aria-label="Back to contacts"> <ArrowLeft className="h-4 w-4" /> </Button> </Link>apps/web/src/app/(dashboard)/contacts/duplicates/duplicates-list.tsx (1)
28-31: Default pagination object missingtotalCountproperty.Line 106 references
pagination.totalCount, but the default fallback doesn't include it. While the nullish coalescing handles this, adding it to the default improves type consistency.Proposed fix
const { duplicates, pagination } = duplicatesQuery.data ?? { duplicates: [], - pagination: { page: 1, totalPages: 1, hasNext: false, hasPrev: false }, + pagination: { page: 1, totalPages: 1, hasNext: false, hasPrev: false, totalCount: 0 }, };apps/web/src/utils/sanitize-html.ts (1)
11-80: Consider adding explicit URI protocol restrictions for defense in depth.While DOMPurify blocks dangerous protocols like
javascript:by default, explicitly configuringALLOWED_URI_REGEXPadds an extra layer of protection and makes the security intent clearer.🔒 Proposed enhancement
return DOMPurify.sanitize(html, { ALLOWED_TAGS: [ "a", // ... rest of tags ], ALLOWED_ATTR: [ "href", "src", // ... rest of attributes ], ALLOW_DATA_ATTR: false, ADD_ATTR: ["target"], + ALLOWED_URI_REGEXP: /^(?:(?:https?|mailto):)/i, });apps/web/src/app/(dashboard)/templates/[templateId]/edit/template-preview.tsx (3)
111-116: Security: Consider sanitizing HTML before rendering in iframe.The HTML returned from
/api/to-htmlis rendered directly viasrcDocwithout sanitization. Per the AI summary, asanitizeHtmlutility using DOMPurify was added in this PR. Consider using it here to prevent potential XSS if the template content contains malicious scripts.Additionally, the
sandbox="allow-same-origin"attribute allows the iframe content to access same-origin resources. For untrusted HTML previews, consider usingsandbox=""(empty) or at minimum removingallow-same-originto provide stronger isolation.Suggested approach
+import { sanitizeHtml } from "~/utils/sanitize-html";Then in the component:
- <iframe - srcDoc={html} - className="w-full h-full" - sandbox="allow-same-origin" - title="Email Preview" - /> + <iframe + srcDoc={sanitizeHtml(html)} + className="w-full h-full" + sandbox="" + title="Email Preview" + />
30-35: Potential race condition when opening dialog rapidly.If the user rapidly opens/closes the dialog, the async
loadPreview()call from a previous open could complete after closing, updating state on an unmounted or closed dialog. Consider adding an abort mechanism or checking if the dialog is still open before setting state.Suggested fix using AbortController
+import { useState, useRef, useEffect } from "react"; -import { useState } from "react"; export function TemplatePreview({ json, subject }: TemplatePreviewProps) { const [open, setOpen] = useState(false); const [html, setHtml] = useState<string | null>(null); const [loading, setLoading] = useState(false); const [viewMode, setViewMode] = useState<ViewMode>("desktop"); + const abortControllerRef = useRef<AbortController | null>(null); const handleOpenChange = async (isOpen: boolean) => { setOpen(isOpen); + if (!isOpen && abortControllerRef.current) { + abortControllerRef.current.abort(); + } if (isOpen && json) { await loadPreview(); } }; const loadPreview = async () => { if (!json) { toast.error("No template content to preview"); return; } + abortControllerRef.current?.abort(); + abortControllerRef.current = new AbortController(); setLoading(true); try { const response = await fetch("/api/to-html", { method: "POST", headers: { "Content-Type": "application/json", }, body: JSON.stringify(json), + signal: abortControllerRef.current.signal, });
59-60: Swallowed error loses debugging context.The caught error is discarded and replaced with a generic toast message. Consider logging the error for debugging purposes.
Suggested fix
} catch (error) { + console.error("Failed to load preview:", error); toast.error("Failed to load preview"); } finally {apps/web/src/server/api/routers/email.ts (2)
90-91: Date input lacks format validation.The
dateFromanddateTofields accept any string but are used directly withnew Date(). Invalid date strings will produceInvalid Dateobjects, potentially causing unexpected query behavior. Consider using Zod's date validation or a custom refinement.Suggested fix
- dateFrom: z.string().optional().nullable(), - dateTo: z.string().optional().nullable(), + dateFrom: z.string().datetime().optional().nullable(), + dateTo: z.string().datetime().optional().nullable(),Or if you expect YYYY-MM-DD format specifically:
- dateFrom: z.string().optional().nullable(), - dateTo: z.string().optional().nullable(), + dateFrom: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).optional().nullable(), + dateTo: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).optional().nullable(),Also applies to: 142-143
113-114: Date range end-time handling assumes specific input format.The code appends
"T23:59:59.999Z"todateToto make the range inclusive of the entire day. This works correctly only ifdateTois inYYYY-MM-DDformat. If a full ISO timestamp is passed, the result would be malformed (e.g.,"2024-01-15T00:00:00Z" + "T23:59:59.999Z").Consider parsing the date first and setting the end of day programmatically:
Suggested fix using date-fns
+import { endOfDay } from "date-fns"; // In the query: -${input.dateTo ? Prisma.sql`AND "createdAt" <= ${new Date(input.dateTo + "T23:59:59.999Z")}` : Prisma.sql``} +${input.dateTo ? Prisma.sql`AND "createdAt" <= ${endOfDay(new Date(input.dateTo))}` : Prisma.sql``}Also applies to: 188-189
apps/web/src/server/api/routers/contacts.ts (2)
165-231:findDuplicates: likely needs an index plan (can become a hot path).
This groups across all contacts for a team and joinsContactBookeach request; on large teams this will get expensive quickly. At minimum, ensure you have supporting indexes (e.g.,Contact(contactBookId, email)andContactBook(teamId)), and consider a cached/materialized approach if this is used frequently.
233-259:getDuplicateDetails: consider selecting fields + bounding result size.
db.contact.findMany()withoutselectreturns all columns; if the UI only needs a subset, preferselectto reduce payload/PII surface. Also consider a reasonable cap/order if an email can appear in many books.apps/web/src/app/(dashboard)/settings/layout.tsx (1)
7-7: Remove ineffectivedynamicexport from client component.The
dynamic = "force-static"export has no effect in client components (marked with"use client"). This directive is only meaningful for Server Components in Next.js.♻️ Proposed fix
"use client"; import { useTeam } from "~/providers/team-context"; import { SettingsNavButton } from "../dev-settings/settings-nav-button"; import { isCloud } from "~/utils/common"; -export const dynamic = "force-static"; - export default function ApiKeysPage({apps/web/src/app/(dashboard)/campaigns/page.tsx (1)
6-6: Use barrel export for Button import.Line 5 uses the barrel export from
@usesend/uifor H1, but Line 6 imports Button from the full path. For consistency, prefer barrel exports.♻️ Proposed fix
import CampaignList from "./campaign-list"; import CreateCampaign from "./create-campaign"; -import { H1 } from "@usesend/ui"; -import { Button } from "@usesend/ui/src/button"; +import { H1, Button } from "@usesend/ui"; import { BarChart3 } from "lucide-react"; import Link from "next/link";apps/web/src/app/(dashboard)/dev-settings/webhooks/delete-webhook.tsx (1)
40-43: Consider adding an accessible label to the delete button.The delete trigger button contains only an icon without text, which may impact accessibility. Consider adding an
aria-labelattribute.♻️ Proposed fix
<AlertDialogTrigger asChild> - <Button variant="ghost" size="sm"> + <Button variant="ghost" size="sm" aria-label={`Delete webhook ${webhook.name}`}> <Trash2 className="h-4 w-4 text-destructive" /> </Button> </AlertDialogTrigger>apps/web/src/app/(dashboard)/campaigns/compare/campaign-selector.tsx (1)
58-60: Consider using strict equality for maximum badge.The condition
selectedIds.length >= 5could be simplified to=== 5since the maximum is exactly 5 campaigns, making the intent clearer.♻️ Proposed fix
- {selectedIds.length >= 5 && ( + {selectedIds.length === 5 && ( <Badge variant="secondary">Maximum 5 campaigns</Badge> )}apps/web/src/server/api/routers/campaign.ts (1)
390-392: Simplify delivery rate calculation.The
?? 1fallback in the division is unreachable because the condition already ensuressent > 0. This can be simplified for clarity.♻️ Proposed fix
deliveryRate: - (campaign.sent ?? 0) > 0 - ? (delivered / (campaign.sent ?? 1)) * 100 + campaign.sent && campaign.sent > 0 + ? (delivered / campaign.sent) * 100 : 0,apps/web/src/server/api/routers/api.ts (1)
2-2: Remove unusedPrismaimport.The
Prismanamespace is imported but not used in this file. OnlyApiPermissionis referenced.🧹 Suggested fix
-import { ApiPermission, Prisma } from "@prisma/client"; +import { ApiPermission } from "@prisma/client";apps/web/src/app/(dashboard)/dev-settings/usage/page.tsx (2)
23-23: Remove unusedAlertCircleimport.
AlertCircleis imported but not used in this component.🧹 Suggested fix
-import { BarChart3, Mail, CheckCircle, XCircle, AlertCircle } from "lucide-react"; +import { BarChart3, Mail, CheckCircle, XCircle } from "lucide-react";
153-155: MovemaxTotalcalculation outside the map for better performance.
maxTotalis recalculated on every iteration. While the impact is minimal for 90 data points, it's cleaner to compute once.♻️ Suggested fix
<div className="h-[200px] flex items-end gap-1"> - {data.daily.map((day, idx) => { - const maxTotal = Math.max(...data.daily.map((d) => d.total)); + {(() => { + const maxTotal = Math.max(...data.daily.map((d) => d.total)); + return data.daily.map((day, idx) => { const height = maxTotal > 0 ? (day.total / maxTotal) * 100 : 0; // ... rest of the map body + }); + })()}Or extract
maxTotalbefore the JSX:const maxTotal = data.daily.length > 0 ? Math.max(...data.daily.map((d) => d.total)) : 0;apps/web/src/app/(dashboard)/settings/reports/delete-report.tsx (1)
53-58: Consider preventing dialog close until mutation completes.The current implementation closes the AlertDialog immediately when the Delete button is clicked, then runs the mutation. If the mutation fails, the error toast appears but the dialog is already closed, which might be slightly confusing.
For improved UX, consider using a controlled AlertDialog that stays open during the mutation:
♻️ Optional improvement for better mutation feedback
+ const [open, setOpen] = useState(false); + const deleteMutation = api.scheduledReport.delete.useMutation({ onSuccess: () => { toast.success("Report deleted"); utils.scheduledReport.list.invalidate(); + setOpen(false); }, onError: (error) => { toast.error(error.message); }, }); return ( - <AlertDialog> + <AlertDialog open={open} onOpenChange={setOpen}> <AlertDialogTrigger asChild> <Button variant="ghost" size="sm"> <Trash2 className="h-4 w-4 text-destructive" /> </Button> </AlertDialogTrigger> <AlertDialogContent> <AlertDialogHeader> <AlertDialogTitle>Delete scheduled report</AlertDialogTitle> <AlertDialogDescription> Are you sure you want to delete "{report.name}"? This action cannot be undone. </AlertDialogDescription> </AlertDialogHeader> <AlertDialogFooter> <AlertDialogCancel>Cancel</AlertDialogCancel> <AlertDialogAction - onClick={() => deleteMutation.mutate({ id: report.id })} + onClick={(e) => { + e.preventDefault(); + deleteMutation.mutate({ id: report.id }); + }} className="bg-destructive text-destructive-foreground hover:bg-destructive/90" + disabled={deleteMutation.isPending} > {deleteMutation.isPending ? "Deleting..." : "Delete"} </AlertDialogAction> </AlertDialogFooter> </AlertDialogContent> </AlertDialog>apps/web/src/app/(dashboard)/campaigns/compare/page.tsx (1)
14-24: Add user feedback when 5-campaign limit is reached.When a user tries to select a sixth campaign, the function silently returns without any indication. This could confuse users who don't realize they've hit the limit.
♻️ Proposed enhancement for better UX
Add a toast import at the top:
+"use client"; + +import { useState } from "react"; +import { H1 } from "@usesend/ui"; +import { Button } from "@usesend/ui/src/button"; +import { ArrowLeft } from "lucide-react"; +import Link from "next/link"; ++import { toast } from "@usesend/ui/src/toaster"; +import CampaignSelector from "./campaign-selector"; +import ComparisonView from "./comparison-view";Then update the handler:
const handleToggle = (id: string) => { setSelectedIds((prev) => { if (prev.includes(id)) { return prev.filter((i) => i !== id); } if (prev.length >= 5) { + toast.error("Maximum 5 campaigns can be selected for comparison"); return prev; } return [...prev, id]; }); };apps/web/src/app/(dashboard)/dev-settings/webhooks/add-webhook.tsx (1)
127-162: Consider matching dot count to actual secret length.Line 133 hardcodes 40 dots to obscure the secret, which may not match the actual secret length. For better UX consistency, consider using
secret.lengthinstead.♻️ Optional improvement
- {Array.from({ length: 40 }).map((_, index) => ( + {Array.from({ length: secret.length }).map((_, index) => ( <div key={index} className="w-1 h-1 bg-muted-foreground rounded-lg" /> ))}apps/web/src/app/(dashboard)/contacts/[contactBookId]/segments/create-segment.tsx (2)
28-34: Avoidfilters as anyby aligning the client Filter type with the router input.Right now you’re bypassing TRPC’s input typing (Line 98-101, Line 150), which makes it easy to ship filters that the server will ignore or misinterpret.
Proposed change (typed Filter + remove `any`)
interface Filter { - field: string; - operator: string; - value?: string | boolean; + field: + | "email" + | "firstName" + | "lastName" + | "subscribed" + | "createdAt" + | "property"; + operator: + | "equals" + | "not_equals" + | "contains" + | "not_contains" + | "starts_with" + | "ends_with" + | "is_empty" + | "is_not_empty" + | "greater_than" + | "less_than" + | "is_true" + | "is_false"; + value?: string | boolean | number; propertyKey?: string; } const previewQuery = api.segment.preview.useQuery( - { contactBookId, filters: filters as any, limit: 5 }, + { contactBookId, filters, limit: 5 }, { enabled: filters.length > 0 }, ); const handleSubmit = () => { @@ createMutation.mutate({ contactBookId, name, description: description || undefined, - filters: filters as any, + filters, }); };Also applies to: 98-101, 146-151
189-257: Don’t usekey={index}for editable filter rows (can shuffle input state on remove).When you delete a middle row, React can reuse DOM/state for later rows, swapping values unexpectedly.
Proposed change (stable id per filter)
interface Filter { + id: string; field: @@ } const [filters, setFilters] = useState<Filter[]>([ - { field: "subscribed", operator: "is_true" }, + { id: crypto.randomUUID(), field: "subscribed", operator: "is_true" }, ]); const resetForm = () => { @@ - setFilters([{ field: "subscribed", operator: "is_true" }]); + setFilters([{ id: crypto.randomUUID(), field: "subscribed", operator: "is_true" }]); }; const addFilter = () => { - setFilters([...filters, { field: "email", operator: "contains", value: "" }]); + setFilters([ + ...filters, + { id: crypto.randomUUID(), field: "email", operator: "contains", value: "" }, + ]); }; @@ - {filters.map((filter, index) => ( + {filters.map((filter, index) => ( <div - key={index} + key={filter.id} className="flex items-center gap-2 p-3 border rounded-lg bg-muted/30" >apps/web/src/app/(dashboard)/settings/reports/edit-report.tsx (1)
101-118: When mutating recipients, passshouldValidate/shouldDirtyto keep form state consistent.Without it, you can end up with stale validation messages / disabled submit logic in some RHF setups.
Proposed change
if (!current.includes(email)) { - form.setValue("recipients", [...current, email]); + form.setValue("recipients", [...current, email], { + shouldValidate: true, + shouldDirty: true, + }); } @@ form.setValue( "recipients", - current.filter((e) => e !== email) + current.filter((e) => e !== email), + { shouldValidate: true, shouldDirty: true }, );apps/web/src/server/api/routers/scheduled-report.ts (1)
22-68: Timezone is persisted but ignored in scheduling (nextSendAt likely wrong for non-UTC teams).If
nextSendAtis meant to be the actual UTC moment the job runner should send, the calculation needs to interprethour/dayOfWeek/dayOfMonthininput.timezone(not server local time).Also applies to: 98-135
apps/web/src/server/service/webhook-service.ts (1)
143-143: Avoidas anytype assertion.Casting to
any(line 143) bypasses TypeScript's type checking. The Prisma schema likely expects a specific JSON type for the payload field.♻️ Check Prisma schema and use proper typing
If the Prisma field is defined as
Json, you can use:- payload: payload as any, + payload: payload as Prisma.JsonObject,Or if it's
JsonValue:- payload: payload as any, + payload: payload,Verify the Prisma schema definition for
WebhookDelivery.payloadto determine the correct type.
| import AddContactBook from "./add-contact-book"; | ||
| import ContactBooksList from "./contact-books-list"; | ||
| import { H1 } from "@usesend/ui"; | ||
| import { Button } from "@usesend/ui/src/button"; |
There was a problem hiding this comment.
Fix import path inconsistency.
The Button import uses a deep path @usesend/ui/src/button while other imports from the same package use the base path @usesend/ui (e.g., H1 on line 5). This inconsistency could cause bundling issues and violates typical package consumption patterns.
📦 Proposed fix
-import { Button } from "@usesend/ui/src/button";
+import { Button } from "@usesend/ui";📝 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.
| import { Button } from "@usesend/ui/src/button"; | |
| import { Button } from "@usesend/ui"; |
🤖 Prompt for AI Agents
In @apps/web/src/app/(dashboard)/contacts/page.tsx at line 6, The Button import
in page.tsx uses a deep path ("@usesend/ui/src/button") which is inconsistent
with other imports (e.g., H1) from "@usesend/ui"; update the import for Button
to import from the package root ("@usesend/ui") to match the other imports and
avoid bundling/consumption issues, ensuring you only change the import statement
that references Button and leave the rest of the file intact.
| export function DomainReputationBadge({ domainId }: { domainId: number }) { | ||
| const { data: metrics, isLoading } = | ||
| api.dashboard.reputationMetricsData.useQuery({ | ||
| domain: domainId, | ||
| }); | ||
|
|
||
| if (isLoading) { | ||
| return ( | ||
| <div className="h-6 w-16 bg-muted/50 animate-pulse rounded-md" /> | ||
| ); | ||
| } | ||
|
|
||
| if (!metrics || metrics.delivered === 0) { | ||
| return ( | ||
| <div className="text-xs text-muted-foreground px-2 py-1 rounded-md bg-muted/30"> | ||
| No data | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Add error state handling for the query.
The component handles loading and empty states but doesn't handle query errors. If api.dashboard.reputationMetricsData fails, the component will remain in loading state or potentially crash.
🛡️ Add error handling
- const { data: metrics, isLoading } =
+ const { data: metrics, isLoading, isError } =
api.dashboard.reputationMetricsData.useQuery({
domain: domainId,
});
if (isLoading) {
return (
<div className="h-6 w-16 bg-muted/50 animate-pulse rounded-md" />
);
}
+ if (isError) {
+ return (
+ <div className="text-xs text-destructive px-2 py-1 rounded-md bg-destructive/10">
+ Error
+ </div>
+ );
+ }
+
if (!metrics || metrics.delivered === 0) {📝 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 function DomainReputationBadge({ domainId }: { domainId: number }) { | |
| const { data: metrics, isLoading } = | |
| api.dashboard.reputationMetricsData.useQuery({ | |
| domain: domainId, | |
| }); | |
| if (isLoading) { | |
| return ( | |
| <div className="h-6 w-16 bg-muted/50 animate-pulse rounded-md" /> | |
| ); | |
| } | |
| if (!metrics || metrics.delivered === 0) { | |
| return ( | |
| <div className="text-xs text-muted-foreground px-2 py-1 rounded-md bg-muted/30"> | |
| No data | |
| </div> | |
| ); | |
| } | |
| export function DomainReputationBadge({ domainId }: { domainId: number }) { | |
| const { data: metrics, isLoading, isError } = | |
| api.dashboard.reputationMetricsData.useQuery({ | |
| domain: domainId, | |
| }); | |
| if (isLoading) { | |
| return ( | |
| <div className="h-6 w-16 bg-muted/50 animate-pulse rounded-md" /> | |
| ); | |
| } | |
| if (isError) { | |
| return ( | |
| <div className="text-xs text-destructive px-2 py-1 rounded-md bg-destructive/10"> | |
| Error | |
| </div> | |
| ); | |
| } | |
| if (!metrics || metrics.delivered === 0) { | |
| return ( | |
| <div className="text-xs text-muted-foreground px-2 py-1 rounded-md bg-muted/30"> | |
| No data | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In @apps/web/src/app/(dashboard)/domains/domain-reputation-badge.tsx around
lines 43 - 61, The DomainReputationBadge component lacks handling for query
errors from api.dashboard.reputationMetricsData.useQuery; update the hook call
to destructure error and/or isError (e.g., { data: metrics, isLoading, isError,
error }) and add a branch that returns a small error UI when isError (or error
is truthy) — for example a compact "Error" or icon with text and optional
tooltip containing error.message — so the component doesn't hang or crash on
failed requests.
| const createReportSchema = z.object({ | ||
| name: z.string().min(1, "Name is required"), | ||
| recipients: z.array(z.string().email("Invalid email address")).min(1, "At least one recipient is required"), | ||
| frequency: z.enum(["DAILY", "WEEKLY", "MONTHLY"]), | ||
| dayOfWeek: z.string().optional(), | ||
| dayOfMonth: z.string().optional(), | ||
| hour: z.string(), | ||
| timezone: z.string(), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Strengthen schema validation for frequency-dependent fields.
The schema marks dayOfWeek and dayOfMonth as optional, but they're required for WEEKLY and MONTHLY frequencies respectively. Lines 112-113 use fallbacks (?? "1"), which could mask missing values rather than catching validation errors early.
♻️ Enhanced schema with conditional validation
const createReportSchema = z.object({
name: z.string().min(1, "Name is required"),
recipients: z.array(z.string().email("Invalid email address")).min(1, "At least one recipient is required"),
frequency: z.enum(["DAILY", "WEEKLY", "MONTHLY"]),
dayOfWeek: z.string().optional(),
dayOfMonth: z.string().optional(),
hour: z.string(),
timezone: z.string(),
-});
+}).refine(
+ (data) => {
+ if (data.frequency === "WEEKLY" && !data.dayOfWeek) return false;
+ if (data.frequency === "MONTHLY" && !data.dayOfMonth) return false;
+ return true;
+ },
+ {
+ message: "Day of week is required for weekly reports, day of month for monthly reports",
+ }
+);Then remove the fallback operators on lines 112-113:
- dayOfWeek: values.frequency === "WEEKLY" ? parseInt(values.dayOfWeek ?? "1") : undefined,
- dayOfMonth: values.frequency === "MONTHLY" ? parseInt(values.dayOfMonth ?? "1") : undefined,
+ dayOfWeek: values.frequency === "WEEKLY" ? parseInt(values.dayOfWeek!) : undefined,
+ dayOfMonth: values.frequency === "MONTHLY" ? parseInt(values.dayOfMonth!) : undefined,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @apps/web/src/app/(dashboard)/settings/reports/create-report.tsx around lines
52 - 60, createReportSchema currently marks dayOfWeek and dayOfMonth optional
but must enforce them when frequency is WEEKLY or MONTHLY; update
createReportSchema to add a conditional validation (using .superRefine or
refinements) that errors if frequency === "WEEKLY" and dayOfWeek is
missing/empty, or if frequency === "MONTHLY" and dayOfMonth is missing/empty,
and validate their formats if needed; then remove the fallback operators (the
`?? "1"` usages) where the form values are read/submitted so missing values fail
schema validation instead of being masked.
| export async function resendEmail(emailId: string, teamId: number) { | ||
| const originalEmail = await db.email.findUnique({ | ||
| where: { id: emailId, teamId }, | ||
| }); | ||
|
|
||
| if (!originalEmail) { | ||
| throw new UnsendApiError({ | ||
| code: "NOT_FOUND", | ||
| message: "Email not found", | ||
| }); | ||
| } | ||
|
|
||
| // Only allow resending failed or bounced emails | ||
| const allowedStatuses = ["FAILED", "BOUNCED", "REJECTED", "COMPLAINED"]; | ||
| if (!allowedStatuses.includes(originalEmail.latestStatus)) { | ||
| throw new UnsendApiError({ | ||
| code: "BAD_REQUEST", | ||
| message: `Cannot resend email with status ${originalEmail.latestStatus}. Only failed, bounced, rejected, or complained emails can be resent.`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Resend bypasses suppression checks.
The resendEmail function copies recipients directly from the original email without checking if any addresses have been added to the suppression list since the original send attempt. This could result in sending to suppressed addresses.
Consider adding suppression checks similar to sendEmail:
Suggested approach
+ // Check for suppressed emails before resending
+ const toEmails = originalEmail.to;
+ const ccEmails = originalEmail.cc || [];
+ const bccEmails = originalEmail.bcc || [];
+
+ const allEmailsToCheck = [
+ ...new Set([...toEmails, ...ccEmails, ...bccEmails]),
+ ];
+
+ const suppressionResults = await SuppressionService.checkMultipleEmails(
+ allEmailsToCheck,
+ teamId
+ );
+
+ const filteredToEmails = toEmails.filter(
+ (email) => !suppressionResults[email]
+ );
+
+ if (filteredToEmails.length === 0) {
+ throw new UnsendApiError({
+ code: "BAD_REQUEST",
+ message: "All recipients are now suppressed. Cannot resend.",
+ });
+ }Committable suggestion skipped: line range outside the PR's diff.
apps/web/src/utils/sanitize-html.ts
Outdated
| import DOMPurify from "dompurify"; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add "use client" directive to prevent server-side import errors.
DOMPurify requires a DOM implementation and will fail if imported in a server component or during SSR. Add the "use client" directive at the top of this file to ensure it's only bundled for client-side execution.
🔧 Proposed fix
+"use client";
+
import DOMPurify from "dompurify";📝 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.
| import DOMPurify from "dompurify"; | |
| "use client"; | |
| import DOMPurify from "dompurify"; | |
🤖 Prompt for AI Agents
In @apps/web/src/utils/sanitize-html.ts around lines 1 - 2, This file imports
DOMPurify which requires a browser DOM and will break during SSR; add the "use
client" directive as the very first line of apps/web/src/utils/sanitize-html.ts
so the module (and the DOMPurify import) is only evaluated on the client, then
keep the existing import of DOMPurify and any sanitize function (e.g.,
sanitizeHtml) unchanged.
…d-time optimization - A/B Testing: Multi-variant testing for campaigns with winner selection - Audit Logging: Track all team activity for compliance - Email Automation Sequences: Drip campaigns with email, delay, condition, and goal steps - Send-Time Optimization: ML-based optimal send time calculation per contact New files: - Schema models for ABTest, AuditLog, AutomationSequence, SequenceStep, SequenceEnrollment, ContactEngagementPattern, TeamSendTimeSettings - tRPC routers: ab-test, audit-log, sequence, send-time - UI pages: /campaigns/[id]/ab-test, /sequences, /settings/activity, /settings/send-time - Checkbox UI component Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- SEC-001 (XSS): Replace dompurify with isomorphic-dompurify for SSR safety - SEC-002 (SSRF): Add webhook URL validation to block internal IPs, localhost, and cloud metadata endpoints - SEC-003 (Secret Exposure): Fetch webhook secrets from DB in worker instead of passing through job queue data Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
14 issues found across 24 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/prisma/schema.prisma">
<violation number="1" location="apps/web/prisma/schema.prisma:885">
P2: Missing `@@index([emailId])` on `EmailEngagementEvent` model. The similar `EmailEvent` model includes this index. Queries filtering engagement events by `emailId` will be slow without it.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/sequences/create-sequence-dialog.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/sequences/create-sequence-dialog.tsx:54">
P2: Query executes even when dialog is closed. Add `enabled: open` option to defer the fetch until the dialog is actually opened.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/sequences/[sequenceId]/page.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/sequences/[sequenceId]/page.tsx:179">
P2: Step deletion lacks confirmation dialog unlike sequence deletion. This inconsistency could lead to accidental data loss since steps may contain email content and configuration. Consider adding a confirmation dialog similar to the one used for sequence deletion at lines 429-463.</violation>
</file>
<file name="apps/web/src/server/api/routers/sequence.ts">
<violation number="1" location="apps/web/src/server/api/routers/sequence.ts:449">
P0: Missing authorization check: `stepIds` are not validated to belong to the specified `sequenceId`. An attacker could manipulate steps from other sequences by passing arbitrary stepIds. Add `sequenceId` to the update where clause to prevent cross-sequence manipulation.</violation>
<violation number="2" location="apps/web/src/server/api/routers/sequence.ts:556">
P1: Missing authorization check: `contactIds` are not validated to belong to the team's contact books. Consider validating that all contacts belong to contact books owned by the team before creating enrollments.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/sequences/page.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/sequences/page.tsx:101">
P2: Missing error state handling for the query. When `sequencesQuery.isError` is true, users see an empty table with no indication that an error occurred. Consider adding an error state between the loading check and the empty state check.</violation>
</file>
<file name="apps/web/src/server/api/routers/webhook.ts">
<violation number="1" location="apps/web/src/server/api/routers/webhook.ts:15">
P1: SSRF protection is vulnerable to DNS rebinding attacks. The validation checks the URL hostname string but not the resolved IP address. An attacker can register a domain resolving to internal IPs (e.g., 169.254.169.254) to bypass all hostname checks. For robust protection, consider resolving the hostname and validating the IP at request time, or use a library that handles DNS rebinding protection.</violation>
<violation number="2" location="apps/web/src/server/api/routers/webhook.ts:41">
P1: SSRF protection can be bypassed using alternative IP address formats. The validation only handles simple dotted-decimal notation but attackers can use decimal (`2130706433` → 127.0.0.1), hex (`0x7f000001`), octal (`0177.0.0.1`), or short form (`127.1`) notations to bypass internal IP blocking. Consider using Node.js `net.isIP()` to detect IPs, then parse with `new URL()` and validate the resolved address, or use an established SSRF protection library.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/settings/send-time/page.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/settings/send-time/page.tsx:78">
P2: No validation that `defaultHourStart` is less than `defaultHourEnd`. Users can set an invalid time range (e.g., 5 PM to 9 AM) which could cause unexpected behavior in send time optimization.</violation>
<violation number="2" location="apps/web/src/app/(dashboard)/settings/send-time/page.tsx:102">
P2: Missing error state handling for `settingsQuery`. If the API request fails, the component will render without proper error feedback, potentially showing stale or undefined data.</violation>
</file>
<file name="apps/web/src/server/api/routers/audit-log.ts">
<violation number="1" location="apps/web/src/server/api/routers/audit-log.ts:213">
P2: The export query has no limit, which could cause memory exhaustion for large date ranges. Consider adding a maximum limit or implementing pagination/streaming for exports.</violation>
<violation number="2" location="apps/web/src/server/api/routers/audit-log.ts:279">
P1: CSV generation doesn't properly escape values containing commas, quotes, or newlines. The `JSON.stringify(log.details)` will contain commas that break CSV parsing. Fields should be wrapped in quotes and internal quotes should be escaped.</violation>
</file>
<file name="apps/web/src/server/api/routers/send-time.ts">
<violation number="1" location="apps/web/src/server/api/routers/send-time.ts:179">
P2: Missing query limit: unlike `getOptimalTimeForContacts` which limits to 10,000 contacts, this query fetches all contacts without a limit, potentially causing memory issues for large contact books.</violation>
<violation number="2" location="apps/web/src/server/api/routers/send-time.ts:314">
P1: Race condition: when updating `totalOpens`/`totalClicks`, the non-incremented field is set to a stale value read earlier, potentially overwriting concurrent updates. Use conditional spreading to only include the field being incremented.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| {sequencesQuery.isLoading ? ( | ||
| <TableRow> | ||
| <TableCell colSpan={7} className="text-center py-8"> | ||
| <Spinner className="w-5 h-5 mx-auto" /> | ||
| </TableCell> |
There was a problem hiding this comment.
P2: Missing error state handling for the query. When sequencesQuery.isError is true, users see an empty table with no indication that an error occurred. Consider adding an error state between the loading check and the empty state check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/app/(dashboard)/sequences/page.tsx, line 101:
<comment>Missing error state handling for the query. When `sequencesQuery.isError` is true, users see an empty table with no indication that an error occurred. Consider adding an error state between the loading check and the empty state check.</comment>
<file context>
@@ -0,0 +1,233 @@
+ </TableRow>
+ </TableHeader>
+ <TableBody>
+ {sequencesQuery.isLoading ? (
+ <TableRow>
+ <TableCell colSpan={7} className="text-center py-8">
</file context>
| {sequencesQuery.isLoading ? ( | |
| <TableRow> | |
| <TableCell colSpan={7} className="text-center py-8"> | |
| <Spinner className="w-5 h-5 mx-auto" /> | |
| </TableCell> | |
| {sequencesQuery.isLoading ? ( | |
| <TableRow> | |
| <TableCell colSpan={7} className="text-center py-8"> | |
| <Spinner className="w-5 h-5 mx-auto" /> | |
| </TableCell> | |
| </TableRow> | |
| ) : sequencesQuery.isError ? ( | |
| <TableRow> | |
| <TableCell | |
| colSpan={7} | |
| className="text-center py-12 text-destructive" | |
| > | |
| <p>Failed to load sequences. Please try again.</p> | |
| </TableCell> | |
| </TableRow> |
| return `${h}:00 ${ampm}`; | ||
| }; | ||
|
|
||
| if (settingsQuery.isLoading) { |
There was a problem hiding this comment.
P2: Missing error state handling for settingsQuery. If the API request fails, the component will render without proper error feedback, potentially showing stale or undefined data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/app/(dashboard)/settings/send-time/page.tsx, line 102:
<comment>Missing error state handling for `settingsQuery`. If the API request fails, the component will render without proper error feedback, potentially showing stale or undefined data.</comment>
<file context>
@@ -0,0 +1,325 @@
+ return `${h}:00 ${ampm}`;
+ };
+
+ if (settingsQuery.isLoading) {
+ return (
+ <div className="flex items-center justify-center h-64">
</file context>
| } | ||
| }, [settingsQuery.data]); | ||
|
|
||
| const handleSave = () => { |
There was a problem hiding this comment.
P2: No validation that defaultHourStart is less than defaultHourEnd. Users can set an invalid time range (e.g., 5 PM to 9 AM) which could cause unexpected behavior in send time optimization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/app/(dashboard)/settings/send-time/page.tsx, line 78:
<comment>No validation that `defaultHourStart` is less than `defaultHourEnd`. Users can set an invalid time range (e.g., 5 PM to 9 AM) which could cause unexpected behavior in send time optimization.</comment>
<file context>
@@ -0,0 +1,325 @@
+ }
+ }, [settingsQuery.data]);
+
+ const handleSave = () => {
+ updateMutation.mutate({
+ enableOptimization,
</file context>
| getRecent: teamProcedure | ||
| .input(z.object({ limit: z.number().min(5).max(50).default(10) })) | ||
| .query(async ({ ctx: { db, team }, input }) => { | ||
| const logs = await db.auditLog.findMany({ |
There was a problem hiding this comment.
P2: The export query has no limit, which could cause memory exhaustion for large date ranges. Consider adding a maximum limit or implementing pagination/streaming for exports.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/server/api/routers/audit-log.ts, line 213:
<comment>The export query has no limit, which could cause memory exhaustion for large date ranges. Consider adding a maximum limit or implementing pagination/streaming for exports.</comment>
<file context>
@@ -0,0 +1,285 @@
+ getRecent: teamProcedure
+ .input(z.object({ limit: z.number().min(5).max(50).default(10) }))
+ .query(async ({ ctx: { db, team }, input }) => {
+ const logs = await db.auditLog.findMany({
+ where: { teamId: team.id },
+ include: {
</file context>
| const contacts = await db.contact.findMany({ | ||
| where: { | ||
| contactBookId: input.contactBookId, | ||
| contactBook: { teamId: team.id }, | ||
| }, | ||
| select: { id: true }, | ||
| take: 10000, |
There was a problem hiding this comment.
P2: Missing query limit: unlike getOptimalTimeForContacts which limits to 10,000 contacts, this query fetches all contacts without a limit, potentially causing memory issues for large contact books.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/server/api/routers/send-time.ts, line 179:
<comment>Missing query limit: unlike `getOptimalTimeForContacts` which limits to 10,000 contacts, this query fetches all contacts without a limit, potentially causing memory issues for large contact books.</comment>
<file context>
@@ -0,0 +1,458 @@
+ let contactIds = input.contactIds;
+
+ if (input.contactBookId && !contactIds) {
+ const contacts = await db.contact.findMany({
+ where: {
+ contactBookId: input.contactBookId,
</file context>
| const contacts = await db.contact.findMany({ | |
| where: { | |
| contactBookId: input.contactBookId, | |
| contactBook: { teamId: team.id }, | |
| }, | |
| select: { id: true }, | |
| take: 10000, | |
| const contacts = await db.contact.findMany({ | |
| where: { | |
| contactBookId: input.contactBookId, | |
| contactBook: { teamId: team.id }, | |
| }, | |
| select: { id: true }, | |
| take: 10000, | |
| }); |
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In @apps/web/prisma/schema.prisma:
- Around line 748-751: The schema fields yesStepId and noStepId are plain
strings referencing other SequenceStep records but lack foreign-key relations;
add explicit self-referential relations on the SequenceStep model (e.g., fields
yesStepId and noStepId paired with relation fields yesStep and noStep using
named relations like SequenceStep_yes and SequenceStep_no) or, if you prefer not
to change DB schema, implement application-level validation in the SequenceStep
create/update logic to verify referenced step IDs exist and enforce
cascade/constraint behavior; after adding relations, set appropriate referential
actions (onDelete/onUpdate) and run prisma migrate to apply the change and
update any code that reads/writes yesStepId/noStepId to use the new relation
fields (SequenceStep.yesStep, SequenceStep.noStep).
In @apps/web/src/app/(dashboard)/sequences/[sequenceId]/add-step-dialog.tsx:
- Around line 91-128: Update the Zod schema that defines delayDuration to
require a minimum of 1 (e.g., change delayDuration from z.number().optional() to
z.number().min(1).optional() or the equivalent in your schema) and add a runtime
guard in the add handler (handleAdd) to enforce this for DELAY steps: when
stepType === "DELAY" check that delayDuration is a number >= 1, show a toast
error and return if it isn’t, otherwise call addStepMutation.mutate with the
delay fields; this ensures both schema-level and handler-level protection
against zero/negative delays.
In
@apps/web/src/app/(dashboard)/sequences/[sequenceId]/sequence-settings-dialog.tsx:
- Around line 102-114: The updateMutation payload mixes undefined and null for
optional fields causing inconsistency; pick one representation expected by the
backend and apply it uniformly in the updateMutation.mutate call: change
contactBookId to use the same pattern as description/fromEmail/fromName/replyTo
(either `|| undefined` or `|| null`) and ensure any trimming (e.g., name.trim(),
fromEmail.trim()) remains; update the fields in the mutate call (id, name,
description, triggerType, contactBookId, fromEmail, fromName, replyTo,
exitOnUnsubscribe, exitOnGoal, allowReentry) to use the chosen consistent
optional value handling.
- Around line 138-145: The label for the Name field is not associated with its
input; add an id on the Input component (e.g., id="sequence-name") and set the
label's htmlFor to that id so screen readers and click-to-focus work; update the
Input in the sequence-settings-dialog component (the Input with value={name} and
onChange={(e) => setName(e.target.value)}) and ensure any other label/Input
pairs in this dialog follow the same pattern.
In @apps/web/src/app/(dashboard)/settings/activity/page.tsx:
- Around line 153-154: The current use of action.replace("_", " ") only replaces
the first underscore; update all occurrences where the UI renders action (the
action variable inside the SelectItem mapping and the other places noted) to
perform a global underscore-to-space replacement (use a global replace method
such as replaceAll or a replace with a global regex) so multi-underscore action
types like BULK_DELETE_CONTACTS render correctly; change the calls that
reference action.replace("_", " ") (including the SelectItem rendering and the
two other spots mentioned) to use the global replace approach.
In @apps/web/src/app/(dashboard)/settings/send-time/page.tsx:
- Around line 247-290: Add validation to ensure defaultHourStart <
defaultHourEnd: in the component, before saving in handleSave validate the two
state values (defaultHourStart and defaultHourEnd) and return/show an error if
invalid; additionally, prevent invalid selections by filtering the Select
dropdowns (when rendering HOURS for the start Select only include hours <
defaultHourEnd, and for the end Select only include hours > defaultHourStart) or
mark invalid SelectItem options disabled; keep using setDefaultHourStart and
setDefaultHourEnd for updates and ensure the Save button is disabled when the
range is invalid.
In @apps/web/src/server/api/routers/send-time.ts:
- Around line 363-372: The findMany call retrieving all contacts
(db.contact.findMany used to build contacts -> contactIds for
input.contactBookId and team.id) is unbounded and can OOM on large contact
books; change the logic to use pagination or a size cap: either paginate through
results with cursor/limit and process in batches (using limit + skip or
cursor-based queries) to accumulate or stream contactIds, or add a configurable
limit/sample when full processing isn't required; alternatively perform
aggregation on the DB side (avoid fetching all ids) if you only need
counts/patterns. Ensure updates touch the code that consumes contactIds so it
can handle batched/streamed arrays instead of assuming a single full array.
- Around line 307-324: The update to db.contactEngagementPattern.update
currently assigns raw values for non-matching counters which can overwrite
concurrent increments; change totalOpens and totalClicks to use atomic
increments instead (e.g., totalOpens: { increment: input.eventType === "open" ?
1 : 0 } and totalClicks: { increment: input.eventType === "click" ? 1 : 0 }) so
both branches use Prisma's increment operation rather than writing
existingPattern.totalOpens/existingPattern.totalClicks.
In @apps/web/src/server/api/routers/sequence.ts:
- Around line 486-493: The update blindly reorders steps using the provided
input.stepIds without confirming ownership; fetch the sequenceStep records for
input.stepIds (e.g., via db.sequenceStep.findMany with id in input.stepIds),
verify the returned set length equals input.stepIds.length and that every record
has sequenceId === input.sequenceId, and if any mismatch throw an
authorization/bad-request error; only after this validation proceed with the
Promise.all of db.sequenceStep.update calls (optionally wrap in a transaction)
so you can't reorder steps from other sequences.
🧹 Nitpick comments (21)
packages/ui/package.json (1)
34-35: Dependencies added correctly but out of alphabetical order.The new Radix UI dependencies are appropriate for the new Checkbox and AlertDialog components. However, they're not in alphabetical order—
@radix-ui/react-alert-dialogshould come before@radix-ui/react-checkbox.Suggested ordering fix
- "@radix-ui/react-checkbox": "^1.1.4", "@radix-ui/react-alert-dialog": "^1.1.11", + "@radix-ui/react-checkbox": "^1.1.4",apps/web/src/app/(dashboard)/sequences/[sequenceId]/sequence-settings-dialog.tsx (2)
52-65: Consider consolidating state with a form library or useReducer.Managing 11 individual
useStatecalls makes the component harder to maintain and increases the risk of state synchronization bugs. Since@usesend/uialready depends onreact-hook-form, consider using it here for cleaner form state management.
81-94: useEffect may trigger unnecessarily ifsequenceobject reference changes.The dependency on the
sequenceobject means this effect re-runs whenever the parent re-renders with a new object reference, even if the values are identical. Consider using individual properties or memoizing the sequence prop upstream.Alternative approach using stable dependencies
useEffect(() => { if (open) { setName(sequence.name); setDescription(sequence.description || ""); // ... rest of setters } - }, [open, sequence]); + }, [ + open, + sequence.id, // Reset only when a different sequence is opened + ]);apps/web/src/app/(dashboard)/settings/activity/page.tsx (2)
56-72: Download link creation could be more robust.The current approach creates an anchor, clicks it, then revokes the URL. While this works, the anchor is never appended to the DOM which can cause issues in some browsers/contexts. Consider using
window.openfor data URLs or ensuring the anchor is briefly attached.More robust download pattern
onSuccess: (data) => { const blob = new Blob([data.data], { type: data.format === "csv" ? "text/csv" : "application/json", }); const url = URL.createObjectURL(blob); const a = document.createElement("a"); a.href = url; a.download = `audit-log-${format(new Date(), "yyyy-MM-dd")}.${data.format}`; + document.body.appendChild(a); a.click(); + document.body.removeChild(a); URL.revokeObjectURL(url); toast.success("Audit log exported"); },
233-237: Consider using Next.js Image component for user avatars.Using a plain
<img>tag bypasses Next.js image optimization. For user-uploaded profile images from external sources, consider usingnext/imagewith appropriateremotePatternsconfiguration.apps/web/src/server/api/routers/audit-log.ts (2)
51-51: Variable shadowing:dbfrom context shadows module import.The
dbis destructured fromctxbut is also imported at the module level (line 8). While they likely reference the same instance, this shadowing can cause confusion. Consider removing the module-level import since procedures receivedbvia context.
232-241: Export procedure is a mutation but performs no writes.The
exportprocedure only reads data and formats it for download. Using.mutationinstead of.queryis unconventional and prevents caching. Consider changing to.queryunless there's a specific reason for mutation semantics.Change to query
// Export audit logs (for compliance) - export: teamAdminProcedure + export: teamAdminProcedure .input( z.object({ startDate: z.date(), endDate: z.date(), format: z.enum(["json", "csv"]).default("json"), }) ) - .mutation(async ({ ctx: { db, team }, input }) => { + .query(async ({ ctx: { db, team }, input }) => {apps/web/prisma/schema.prisma (1)
575-590: Webhook secret storage should be documented.The
secretfield stores the HMAC signing key. Consider adding a comment clarifying that:
- Secrets are generated securely (32 random bytes as hex)
- They should never be exposed in API responses (only shown once on creation)
This helps future maintainers understand the security model.
url String - secret String // Used to sign webhook payloads + secret String // HMAC-SHA256 signing key (64 hex chars). Only expose on creation. events WebhookEventType[] // Events to subscribe toapps/web/src/server/api/routers/webhook.ts (1)
12-67: Consider expanding IPv6 and link-local coverage in SSRF validation.The SSRF protection is solid for common cases but has gaps:
- IPv6 private ranges: Only
::1is blocked; missingfc00::/7(unique local),fe80::/10(link-local)- Link-local IPv4:
169.254.0.0/16is blocked only for metadata endpoints, but other link-local IPs could be exploited- DNS rebinding: The check occurs at URL parse time; a malicious DNS could return internal IPs at fetch time
♻️ Suggested improvements
const blockedHostnames = [ "localhost", "127.0.0.1", "0.0.0.0", "::1", "[::1]", ]; + +// Block IPv6 private/local ranges +const ipv6Prefixes = ["fc", "fd", "fe80"]; +if (hostname.startsWith("[")) { + const ipv6 = hostname.slice(1, -1).toLowerCase(); + if (ipv6Prefixes.some(prefix => ipv6.startsWith(prefix))) { + return { valid: false, reason: "Internal IPv6 addresses are not allowed" }; + } +}Consider also resolving the hostname and validating the resolved IP at request time to mitigate DNS rebinding, though this may require changes in the webhook delivery layer.
apps/web/src/app/(dashboard)/settings/send-time/page.tsx (2)
20-45: Consider expanding the timezone list or using a library.The hardcoded TIMEZONES list covers major zones but is limited. Consider using
Intl.supportedValuesOf('timeZone')(where supported) or a library liketimezone-supportfor comprehensive coverage.
68-76: State synchronization may overwrite user edits on refetch.The
useEffectsyncs server data to local state on everysettingsQuery.datachange. If the query refetches (e.g., window refocus) while the user is editing, their unsaved changes will be lost.♻️ Suggested fix
Consider initializing state only once or tracking dirty state:
+const [initialized, setInitialized] = useState(false); + useEffect(() => { - if (settingsQuery.data) { + if (settingsQuery.data && !initialized) { setEnableOptimization(settingsQuery.data.enableOptimization); setDefaultHourStart(settingsQuery.data.defaultHourStart); setDefaultHourEnd(settingsQuery.data.defaultHourEnd); setExcludeDays(settingsQuery.data.excludeDays as number[]); setDefaultTimezone(settingsQuery.data.defaultTimezone); + setInitialized(true); } -}, [settingsQuery.data]); +}, [settingsQuery.data, initialized]);apps/web/src/app/(dashboard)/campaigns/[campaignId]/ab-test/page.tsx (2)
94-96: Consider improving the "not found" error state.The current implementation returns a plain
<div>for the "Campaign not found" case. Consider using a more user-friendly error component with navigation options.💡 Suggested improvement
if (!campaign) { - return <div>Campaign not found</div>; + return ( + <div className="text-center py-12"> + <h3 className="text-lg font-semibold mb-2">Campaign not found</h3> + <p className="text-muted-foreground mb-4"> + The campaign you're looking for doesn't exist or you don't have access. + </p> + <Button asChild variant="outline"> + <Link href="/campaigns">Back to Campaigns</Link> + </Button> + </div> + ); }
109-117: Potential undefined access inupdateVariant.The spread
...updated[index]could cause issues ifindexis somehow out of bounds, though the current UI logic should prevent this.💡 Defensive check
const updateVariant = ( index: number, field: "subject" | "previewText", value: string ) => { const updated = [...variants]; + if (!updated[index]) return; updated[index] = { ...updated[index], [field]: value }; setVariants(updated); };apps/web/src/server/api/routers/send-time.ts (1)
67-86: Consider validatingdefaultHourEnd >= defaultHourStart.The current validation allows
defaultHourEndto be less thandefaultHourStart(e.g., start=17, end=9), which could lead to unexpected behavior in send-time calculations.💡 Add cross-field validation
updateSettings: teamAdminProcedure .input( z.object({ enableOptimization: z.boolean().optional(), defaultHourStart: z.number().min(0).max(23).optional(), defaultHourEnd: z.number().min(0).max(23).optional(), excludeDays: z.array(z.number().min(0).max(6)).optional(), defaultTimezone: z.string().optional(), - }) + }).refine( + (data) => { + if (data.defaultHourStart !== undefined && data.defaultHourEnd !== undefined) { + return data.defaultHourEnd >= data.defaultHourStart; + } + return true; + }, + { message: "End hour must be greater than or equal to start hour" } + ) )apps/web/src/server/api/routers/ab-test.ts (2)
120-131: Variantnameis required by schema but has fallback logic.The input schema requires
name: z.string(), which means empty strings are allowed but undefined is not. The fallbackv.name || String.fromCharCode(...)on line 173 handles empty strings, which is correct. However, consider using.min(1)if names should always be provided, or.optional()if the auto-naming is the intended behavior.💡 Clarify intent
If auto-naming is preferred:
z.object({ - name: z.string(), + name: z.string().optional(), subject: z.string(),If names should be required:
z.object({ - name: z.string(), + name: z.string().min(1), subject: z.string(),
466-477: Consider extracting duplicated metrics calculation.The metrics calculation logic (lines 467-477) is identical to the one in the
getprocedure (lines 93-103). Consider extracting to a helper function for maintainability.♻️ Extract helper function
// Add near the top of the file function calculateVariantMetrics(variant: { delivered: number | null; opened: number | null; clicked: number | null; }) { const delivered = variant.delivered || 0; const opened = variant.opened || 0; const clicked = variant.clicked || 0; return { openRate: delivered > 0 ? (opened / delivered) * 100 : 0, clickRate: delivered > 0 ? (clicked / delivered) * 100 : 0, }; } // Then use in both procedures: const variantsWithMetrics = test.variants.map((variant) => ({ ...variant, ...calculateVariantMetrics(variant), }));apps/web/src/app/(dashboard)/sequences/[sequenceId]/page.tsx (1)
459-464: Delete action doesn't show loading state.The
AlertDialogActioncallsdeleteMutation.mutate()but doesn't disable during pending state or show a loading indicator, which could lead to double-clicks.🔧 Add loading state to delete action
<AlertDialogAction className="bg-destructive text-destructive-foreground hover:bg-destructive/90" onClick={() => deleteMutation.mutate({ id: sequenceId })} + disabled={deleteMutation.isPending} > - Delete + {deleteMutation.isPending ? "Deleting..." : "Delete"} </AlertDialogAction>apps/web/src/server/api/routers/sequence.ts (4)
225-248: Consider validating allowed status transitions.The current implementation allows any status transition (e.g., ARCHIVED → ACTIVE directly). Consider adding a state machine to enforce valid transitions:
- DRAFT → ACTIVE, ARCHIVED
- ACTIVE → PAUSED, ARCHIVED
- PAUSED → ACTIVE, ARCHIVED
- ARCHIVED → (none, or DRAFT for clone)
💡 Add transition validation
const ALLOWED_TRANSITIONS: Record<SequenceStatus, SequenceStatus[]> = { DRAFT: ["ACTIVE", "ARCHIVED"], ACTIVE: ["PAUSED", "ARCHIVED"], PAUSED: ["ACTIVE", "ARCHIVED"], ARCHIVED: [], // or ["DRAFT"] if cloning is allowed }; // In updateStatus mutation: if (!ALLOWED_TRANSITIONS[sequence.status].includes(input.status)) { throw new TRPCError({ code: "BAD_REQUEST", message: `Cannot transition from ${sequence.status} to ${input.status}`, }); }
283-363: Consider validating step type requirements.The procedure accepts all optional fields but doesn't validate that required fields are present for each step type. For example, an EMAIL step should require either
subjectortemplateId, but a DELAY step needsdelayDurationanddelayUnit.💡 Add type-specific validation
// Add before creating the step: if (input.type === "EMAIL" && !input.subject && !input.templateId) { throw new TRPCError({ code: "BAD_REQUEST", message: "Email steps require a subject or template", }); } if (input.type === "DELAY" && (!input.delayDuration || !input.delayUnit)) { throw new TRPCError({ code: "BAD_REQUEST", message: "Delay steps require duration and unit", }); }
441-454: Step reordering after deletion makes N individual queries.The current implementation fetches all remaining steps and updates each one individually with
Promise.all. For sequences with many steps, this could be inefficient. Consider using a transaction or optimizing withupdateManyif possible.💡 Batch update with transaction
// Use a transaction to ensure consistency await db.$transaction(async (tx) => { await tx.sequenceStep.delete({ where: { id: input.stepId } }); // Reorder in a single query using raw SQL or updateMany await tx.sequenceStep.updateMany({ where: { sequenceId: step.sequenceId, order: { gt: step.order }, }, data: { order: { decrement: 1 } }, }); });
554-573: Consider wrapping enrollment creation and counter update in a transaction.The
createManyfor enrollments and theupdatefortotalEnrolledare separate operations. If the counter update fails after enrollments are created, the count will be inaccurate.🔧 Use transaction for atomicity
+ await db.$transaction(async (tx) => { // Create enrollments const now = new Date(); - await db.sequenceEnrollment.createMany({ + await tx.sequenceEnrollment.createMany({ data: contactsToEnroll.map((contactId) => ({ sequenceId: input.sequenceId, contactId, currentStepId: firstStep.id, currentStepOrder: 0, status: "ACTIVE", nextStepAt: now, })), skipDuplicates: true, }); // Update sequence stats - await db.automationSequence.update({ + await tx.automationSequence.update({ where: { id: input.sequenceId }, data: { totalEnrolled: { increment: contactsToEnroll.length } }, }); + });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
apps/web/package.jsonapps/web/prisma/schema.prismaapps/web/src/app/(dashboard)/campaigns/[campaignId]/ab-test/page.tsxapps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/add-step-dialog.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/enroll-contacts-dialog.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/sequence-settings-dialog.tsxapps/web/src/app/(dashboard)/sequences/create-sequence-dialog.tsxapps/web/src/app/(dashboard)/sequences/page.tsxapps/web/src/app/(dashboard)/settings/activity/page.tsxapps/web/src/app/(dashboard)/settings/layout.tsxapps/web/src/app/(dashboard)/settings/send-time/page.tsxapps/web/src/components/AppSideBar.tsxapps/web/src/server/api/root.tsapps/web/src/server/api/routers/ab-test.tsapps/web/src/server/api/routers/audit-log.tsapps/web/src/server/api/routers/send-time.tsapps/web/src/server/api/routers/sequence.tsapps/web/src/server/api/routers/webhook.tsapps/web/src/server/service/webhook-service.tsapps/web/src/utils/sanitize-html.tspackages/ui/package.jsonpackages/ui/src/checkbox.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/utils/sanitize-html.ts
- apps/web/package.json
- apps/web/src/app/(dashboard)/settings/layout.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{tsx,ts,jsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports and ensure proper naming of key components in React/NextJS code
Files:
apps/web/src/server/api/routers/audit-log.tsapps/web/src/server/api/routers/webhook.tsapps/web/src/app/(dashboard)/sequences/page.tsxapps/web/src/app/(dashboard)/campaigns/[campaignId]/ab-test/page.tsxapps/web/src/app/(dashboard)/sequences/create-sequence-dialog.tsxapps/web/src/app/(dashboard)/settings/send-time/page.tsxapps/web/src/components/AppSideBar.tsxapps/web/src/server/api/routers/ab-test.tspackages/ui/src/checkbox.tsxapps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/add-step-dialog.tsxapps/web/src/server/api/routers/send-time.tsapps/web/src/app/(dashboard)/sequences/[sequenceId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/sequence-settings-dialog.tsxapps/web/src/app/(dashboard)/settings/activity/page.tsxapps/web/src/server/api/routers/sequence.tsapps/web/src/app/(dashboard)/sequences/[sequenceId]/enroll-contacts-dialog.tsxapps/web/src/server/api/root.tsapps/web/src/server/service/webhook-service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use TypeScript-first approach with 2-space indent and semicolons enabled by Prettier in apps/web (Next.js), apps/marketing, apps/smtp-server, and all packages
Never use dynamic imports; always import on the top level
Run ESLint via @usesend/eslint-config and ensure no warnings remain before submitting PRs
Files:
apps/web/src/server/api/routers/audit-log.tsapps/web/src/server/api/routers/webhook.tsapps/web/src/app/(dashboard)/sequences/page.tsxapps/web/src/app/(dashboard)/campaigns/[campaignId]/ab-test/page.tsxapps/web/src/app/(dashboard)/sequences/create-sequence-dialog.tsxapps/web/src/app/(dashboard)/settings/send-time/page.tsxapps/web/src/components/AppSideBar.tsxapps/web/src/server/api/routers/ab-test.tspackages/ui/src/checkbox.tsxapps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/add-step-dialog.tsxapps/web/src/server/api/routers/send-time.tsapps/web/src/app/(dashboard)/sequences/[sequenceId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/sequence-settings-dialog.tsxapps/web/src/app/(dashboard)/settings/activity/page.tsxapps/web/src/server/api/routers/sequence.tsapps/web/src/app/(dashboard)/sequences/[sequenceId]/enroll-contacts-dialog.tsxapps/web/src/server/api/root.tsapps/web/src/server/service/webhook-service.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use alias
~/for src imports in apps/web (e.g.,import { x } from "~/utils/x")
Files:
apps/web/src/server/api/routers/audit-log.tsapps/web/src/server/api/routers/webhook.tsapps/web/src/app/(dashboard)/sequences/page.tsxapps/web/src/app/(dashboard)/campaigns/[campaignId]/ab-test/page.tsxapps/web/src/app/(dashboard)/sequences/create-sequence-dialog.tsxapps/web/src/app/(dashboard)/settings/send-time/page.tsxapps/web/src/components/AppSideBar.tsxapps/web/src/server/api/routers/ab-test.tsapps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/add-step-dialog.tsxapps/web/src/server/api/routers/send-time.tsapps/web/src/app/(dashboard)/sequences/[sequenceId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/sequence-settings-dialog.tsxapps/web/src/app/(dashboard)/settings/activity/page.tsxapps/web/src/server/api/routers/sequence.tsapps/web/src/app/(dashboard)/sequences/[sequenceId]/enroll-contacts-dialog.tsxapps/web/src/server/api/root.tsapps/web/src/server/service/webhook-service.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: Prefer to use TRPC for client-server communication unless explicitly asked otherwise in apps/web
Use Prisma for database access in apps/web
Files:
apps/web/src/server/api/routers/audit-log.tsapps/web/src/server/api/routers/webhook.tsapps/web/src/app/(dashboard)/sequences/page.tsxapps/web/src/app/(dashboard)/campaigns/[campaignId]/ab-test/page.tsxapps/web/src/app/(dashboard)/sequences/create-sequence-dialog.tsxapps/web/src/app/(dashboard)/settings/send-time/page.tsxapps/web/src/components/AppSideBar.tsxapps/web/src/server/api/routers/ab-test.tsapps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/add-step-dialog.tsxapps/web/src/server/api/routers/send-time.tsapps/web/src/app/(dashboard)/sequences/[sequenceId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/sequence-settings-dialog.tsxapps/web/src/app/(dashboard)/settings/activity/page.tsxapps/web/src/server/api/routers/sequence.tsapps/web/src/app/(dashboard)/sequences/[sequenceId]/enroll-contacts-dialog.tsxapps/web/src/server/api/root.tsapps/web/src/server/service/webhook-service.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Run Prettier 3 for code formatting on TypeScript, TSX, and Markdown files
Files:
apps/web/src/server/api/routers/audit-log.tsapps/web/src/server/api/routers/webhook.tsapps/web/src/app/(dashboard)/sequences/page.tsxapps/web/src/app/(dashboard)/campaigns/[campaignId]/ab-test/page.tsxapps/web/src/app/(dashboard)/sequences/create-sequence-dialog.tsxapps/web/src/app/(dashboard)/settings/send-time/page.tsxapps/web/src/components/AppSideBar.tsxapps/web/src/server/api/routers/ab-test.tspackages/ui/src/checkbox.tsxapps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/add-step-dialog.tsxapps/web/src/server/api/routers/send-time.tsapps/web/src/app/(dashboard)/sequences/[sequenceId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/sequence-settings-dialog.tsxapps/web/src/app/(dashboard)/settings/activity/page.tsxapps/web/src/server/api/routers/sequence.tsapps/web/src/app/(dashboard)/sequences/[sequenceId]/enroll-contacts-dialog.tsxapps/web/src/server/api/root.tsapps/web/src/server/service/webhook-service.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React components must use PascalCase naming convention (e.g., AppSideBar.tsx)
Files:
apps/web/src/app/(dashboard)/sequences/page.tsxapps/web/src/app/(dashboard)/campaigns/[campaignId]/ab-test/page.tsxapps/web/src/app/(dashboard)/sequences/create-sequence-dialog.tsxapps/web/src/app/(dashboard)/settings/send-time/page.tsxapps/web/src/components/AppSideBar.tsxpackages/ui/src/checkbox.tsxapps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/add-step-dialog.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/sequence-settings-dialog.tsxapps/web/src/app/(dashboard)/settings/activity/page.tsxapps/web/src/app/(dashboard)/sequences/[sequenceId]/enroll-contacts-dialog.tsx
🧠 Learnings (2)
📚 Learning: 2025-11-28T21:14:07.734Z
Learnt from: CR
Repo: usesend/useSend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T21:14:07.734Z
Learning: Applies to apps/web/**/*.{ts,tsx} : Prefer to use TRPC for client-server communication unless explicitly asked otherwise in apps/web
Applied to files:
apps/web/src/server/api/routers/webhook.tsapps/web/src/server/api/routers/send-time.ts
📚 Learning: 2025-11-28T21:13:56.758Z
Learnt from: CR
Repo: usesend/useSend PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-28T21:13:56.758Z
Learning: Applies to **/*.{tsx,ts,jsx,js} : Include all required imports and ensure proper naming of key components in React/NextJS code
Applied to files:
packages/ui/src/checkbox.tsx
🧬 Code graph analysis (11)
apps/web/src/server/api/routers/webhook.ts (4)
apps/web/src/server/api/trpc.ts (1)
teamProcedure(127-152)packages/sdk/types/schema.d.ts (1)
webhooks(1239-1239)apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/service/webhook-service.ts (2)
generateWebhookSecret(49-51)WebhookService(58-328)
apps/web/src/app/(dashboard)/campaigns/[campaignId]/ab-test/page.tsx (1)
packages/ui/src/spinner.tsx (1)
Spinner(4-51)
apps/web/src/app/(dashboard)/settings/send-time/page.tsx (8)
packages/ui/src/toaster.tsx (1)
toast(31-31)packages/ui/src/spinner.tsx (1)
Spinner(4-51)packages/ui/src/card.tsx (5)
Card(74-74)CardContent(79-79)CardHeader(75-75)CardTitle(77-77)CardDescription(78-78)packages/ui/src/calendar.tsx (1)
Calendar(213-213)packages/ui/src/switch.tsx (1)
Switch(29-29)packages/ui/src/select.tsx (5)
Select(150-150)SelectTrigger(153-153)SelectValue(152-152)SelectContent(154-154)SelectItem(156-156)packages/ui/src/badge.tsx (1)
Badge(36-36)packages/ui/src/button.tsx (1)
Button(80-80)
apps/web/src/server/api/routers/ab-test.ts (2)
apps/web/src/server/api/trpc.ts (3)
createTRPCRouter(82-82)teamProcedure(127-152)teamAdminProcedure(154-163)apps/web/src/server/db.ts (1)
db(20-20)
apps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsx (2)
apps/web/src/app/(dashboard)/campaigns/toggle-pause-campaign.tsx (1)
TogglePauseCampaign(10-95)apps/web/src/utils/sanitize-html.ts (1)
sanitizeHtml(7-81)
apps/web/src/app/(dashboard)/sequences/[sequenceId]/add-step-dialog.tsx (7)
packages/ui/src/toaster.tsx (1)
toast(31-31)packages/ui/src/dialog.tsx (6)
Dialog(113-113)DialogContent(118-118)DialogHeader(119-119)DialogTitle(121-121)DialogDescription(122-122)DialogFooter(120-120)packages/ui/src/tabs.tsx (4)
Tabs(55-55)TabsList(55-55)TabsTrigger(55-55)TabsContent(55-55)packages/ui/src/input.tsx (1)
Input(25-25)packages/ui/src/select.tsx (5)
Select(150-150)SelectTrigger(153-153)SelectValue(152-152)SelectContent(154-154)SelectItem(156-156)packages/ui/src/textarea.tsx (1)
Textarea(24-24)packages/ui/src/button.tsx (1)
Button(80-80)
apps/web/src/server/api/routers/send-time.ts (2)
apps/web/src/server/api/trpc.ts (3)
createTRPCRouter(82-82)teamProcedure(127-152)teamAdminProcedure(154-163)apps/web/src/server/db.ts (1)
db(20-20)
apps/web/src/app/(dashboard)/sequences/[sequenceId]/page.tsx (3)
apps/web/src/app/(dashboard)/sequences/[sequenceId]/add-step-dialog.tsx (1)
AddStepDialog(35-345)apps/web/src/app/(dashboard)/sequences/[sequenceId]/sequence-settings-dialog.tsx (1)
SequenceSettingsDialog(45-302)apps/web/src/app/(dashboard)/sequences/[sequenceId]/enroll-contacts-dialog.tsx (1)
EnrollContactsDialog(34-245)
apps/web/src/app/(dashboard)/settings/activity/page.tsx (6)
packages/ui/src/toaster.tsx (1)
toast(31-31)packages/ui/src/button.tsx (1)
Button(80-80)packages/ui/src/select.tsx (5)
Select(150-150)SelectTrigger(153-153)SelectValue(152-152)SelectContent(154-154)SelectItem(156-156)packages/ui/src/table.tsx (6)
Table(109-109)TableHeader(110-110)TableRow(114-114)TableHead(113-113)TableBody(111-111)TableCell(115-115)packages/ui/src/spinner.tsx (1)
Spinner(4-51)packages/ui/src/badge.tsx (1)
Badge(36-36)
apps/web/src/server/api/routers/sequence.ts (2)
apps/web/src/server/api/trpc.ts (3)
createTRPCRouter(82-82)teamProcedure(127-152)teamAdminProcedure(154-163)apps/web/src/server/db.ts (1)
db(20-20)
apps/web/src/server/service/webhook-service.ts (5)
apps/web/src/server/queue/queue-constants.ts (2)
WEBHOOK_DELIVERY_QUEUE(6-6)DEFAULT_QUEUE_OPTIONS(8-13)apps/web/src/server/redis.ts (1)
getRedis(6-13)packages/sdk/types/schema.d.ts (1)
webhooks(1239-1239)apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/logger/log.ts (1)
logger(31-63)
🪛 Biome (2.1.2)
apps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsx
[error] 283-283: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (49)
apps/web/src/components/AppSideBar.tsx (1)
81-98: “Sequences” sidebar entry looks correct; verify route availability/feature-gating.
pathname?.startsWith("/sequences")will correctly highlight nested routes (e.g.,/sequences/123). Please confirm/sequencesis shipped for all users; if it’s tier/flagged, consider hiding it behind the same condition used elsewhere for gated areas.apps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsx (2)
129-140: Verify A/B Test button visibility logic.The A/B Test button is currently visible for all campaign statuses. Consider whether it should be conditionally rendered based on the campaign status, or ensure that the destination route (
/campaigns/${campaign.id}/ab-test) appropriately handles different statuses (e.g., showing existing results for completed campaigns vs. allowing creation for draft/scheduled campaigns).The conditional rendering of Edit vs. Pause/Resume toggle looks correct and works well with the
TogglePauseCampaigncomponent's internal logic.
283-283: HTML sanitization correctly applied.The use of
sanitizeHtml()beforedangerouslySetInnerHTMLis the appropriate security pattern for rendering campaign HTML. The sanitization utility uses isomorphic-dompurify with a comprehensive whitelist of allowed tags and attributes, which mitigates XSS risks.The static analysis warning from Biome is a false positive—it cannot detect that the content is sanitized before injection. This is a known limitation of static analysis tools for this pattern.
packages/ui/src/checkbox.tsx (1)
1-29: LGTM!Well-structured Checkbox component following the established Radix UI wrapper pattern. Proper ref forwarding, className composition with
cn, appropriate accessibility states (focus-visible, disabled), and consistent sizing between root and indicator.apps/web/src/server/service/webhook-service.ts (3)
300-326: Timeout may not be cleared ifresponse.text()throws.If
response.text()throws an exception, execution jumps to the catch block whereclearTimeoutis called, so this is actually handled correctly. However, the early return on line 318 could leave the timeout running ifresponse.text()succeeds but something else throws before the return.Actually, looking more closely—the current structure is fine since
clearTimeoutis in the catch block. Disregard this concern.
241-266: Double increment ofattemptscounter on failure.When delivery fails, the code increments
attempts(line 259), then throws an error (line 265) which triggers BullMQ's retry mechanism. The retry callsdeliverWebhookagain, which will incrementattemptsonce more on the next attempt. This leads toattemptsbeing incremented twice per actual delivery attempt.Suggested fix: only increment on the attempt, not on failure
} catch (error) { const errorMessage = error instanceof Error ? error.message : "Unknown error"; // Update delivery record with failure await db.webhookDelivery.update({ where: { id: deliveryId }, data: { status: WebhookDeliveryStatus.FAILED, response: errorMessage.substring(0, 1000), - attempts: { increment: 1 }, lastAttempt: new Date(), }, }); logger.error({ deliveryId, url, error: errorMessage }, "Webhook delivery error"); throw error; // Re-throw to trigger retry }The increment on line 236 (success path) should handle the count, and you may want to increment at the start of
deliverWebhookbefore the try block to accurately track attempts.Likely an incorrect or invalid review comment.
63-72: Static Queue and Worker instantiation at class definition should be lazy-initialized.The static
workerandwebhookQueueare instantiated when the module loads, establishing a Redis connection immediately on import viagetRedis(). This causes failures in environments without Redis (e.g., build, tests) and creates multiple worker instances in multi-process deployments.Consider moving initialization into a lazy-loaded static method or guarding initialization to only run in the worker process context.
EmailQueueServicedemonstrates a better pattern usinginitializeQueue()for deferred initialization.apps/web/prisma/schema.prisma (2)
371-391: ScheduledReport schema is well-designed.Good use of nullable fields for conditional scheduling (dayOfWeek for weekly, dayOfMonth for monthly), appropriate indexes on teamId and nextSendAt for efficient querying, and sensible defaults.
699-728: AutomationSequence model is comprehensive.Well-structured with appropriate relations, indexes on teamId/status/contactBookId, and useful aggregation fields (totalEnrolled, totalCompleted, totalExited) for dashboard displays without expensive counts.
apps/web/src/server/api/root.ts (1)
17-24: LGTM!The new router imports and wiring follow the established patterns. Import aliases use
~/as per coding guidelines, and router names are consistently camelCased.Also applies to: 47-54
apps/web/src/server/api/routers/webhook.ts (9)
69-79: LGTM!The webhook event types schema is well-defined using Zod's enum validation.
83-100: LGTM!The list procedure correctly scopes to team, includes useful aggregation, and properly redacts the secret to a preview-only format.
103-125: LGTM!Properly scoped to team with appropriate NOT_FOUND handling and consistent secret redaction.
128-163: LGTM!The create procedure applies SSRF validation, generates secure secrets, and correctly returns the full secret only at creation time for the user to save.
166-211: LGTM!The update procedure correctly validates URL changes and maintains proper team authorization.
214-234: LGTM!Secret regeneration follows secure patterns - verifies ownership, generates a new secret, and returns it only once for the user to save.
237-253: LGTM!Delete procedure properly verifies team ownership before deletion.
256-261: LGTM!Test webhook delegation to WebhookService is clean and passes team context for authorization.
264-295: LGTM!Cursor-based pagination is correctly implemented with proper team authorization and the standard "fetch limit+1" pattern to determine if more items exist.
apps/web/src/app/(dashboard)/sequences/page.tsx (2)
1-34: LGTM!Imports follow the
~/alias convention and STATUS_COLORS provides a clean mapping for status badges.
36-233: LGTM!Well-structured page component with proper state management, loading/empty states, and pagination. The filter correctly resets the page when changed.
apps/web/src/app/(dashboard)/settings/send-time/page.tsx (3)
78-94: LGTM!Save and toggle handlers are clean and straightforward.
96-108: LGTM!The
formatHourhelper correctly converts 24-hour to 12-hour format, and the loading state is appropriately displayed.
121-206: LGTM!The insights cards and hourly distribution chart are well-implemented with proper conditional rendering and safe data access patterns.
apps/web/src/app/(dashboard)/sequences/[sequenceId]/add-step-dialog.tsx (3)
1-33: LGTM!Imports and interface definition are clean and follow conventions.
41-89: LGTM!State management is clear and the reset function properly resets all fields. The mutation handlers follow the established toast + invalidate pattern.
130-345: LGTM!The dialog and tab-based UI is well-organized, with appropriate fields for each step type and proper loading state handling.
apps/web/src/app/(dashboard)/sequences/[sequenceId]/enroll-contacts-dialog.tsx (3)
1-32: LGTM!Imports and interface definition are clean and properly typed.
83-113: LGTM!Selection handlers are well-implemented with proper state management and validation before enrollment.
115-245: LGTM!The dialog UI is well-structured with appropriate loading/empty states, scrollable contact list, and dynamic button text showing the selection count.
apps/web/src/app/(dashboard)/sequences/create-sequence-dialog.tsx (3)
1-39: LGTM!Imports follow conventions and TRIGGER_DESCRIPTIONS provides helpful context for users selecting trigger types.
41-87: LGTM!The component has clean state management, proper validation, and good UX with navigation to the newly created sequence on success.
89-190: LGTM!The dialog provides a clear form with helpful trigger type descriptions and appropriate loading states.
apps/web/src/app/(dashboard)/campaigns/[campaignId]/ab-test/page.tsx (2)
1-28: LGTM - Imports are well-organized.The imports follow the coding guidelines using the
~/alias for trpc and importing UI components from@usesend/ui. The use ofusefrom React for unwrapping the params Promise aligns with Next.js 15 patterns.
365-377: No action required. The custom color classesborder-green,ring-green/20, andbg-greenare properly defined in the Tailwind configuration atpackages/tailwind-config/tailwind.config.tsand the CSS variables are declared inpackages/ui/styles/globals.csswith both light and dark mode variants. These are valid Tailwind utilities and follow standard Tailwind color extension patterns.Likely an incorrect or invalid review comment.
apps/web/src/server/api/routers/send-time.ts (3)
10-35:calculateOptimalTimemay return defaults when scores exist but are all zero.The function initializes
maxHourScoreandmaxDayScoreto 0, so if all scores are exactly 0, it returns the defaults (hour 9, day 1). This is likely intentional but worth noting. Additionally, the>comparison means ties go to the first encountered value, which is deterministic but arbitrary.
49-64: LGTM - Clean default handling.The fallback to sensible defaults when no settings exist is a good pattern.
436-442: Same-day scheduling always pushes to next week.When
bestDay === currentDay,daysToAddbecomes 0, then+= 7makes it 7 days. This means if Tuesday is the best day and today is Tuesday, it schedules for next Tuesday instead of today. Consider if this is the intended behavior or if the user should have the option to send today.apps/web/src/server/api/routers/ab-test.ts (4)
1-9: LGTM - Clean imports following guidelines.Imports are well-organized with Prisma enums, TRPC utilities, Zod, and the team procedures using the
~/alias as per coding guidelines.
11-63: LGTM - Well-structured list procedure.Proper pagination, team-scoped filtering, and includes relevant related data. The
Promise.allpattern for parallel queries is efficient.
65-109: LGTM - Proper authorization and metrics calculation.The ownership check correctly validates both existence and team membership before exposing data. Metrics calculation handles the zero-division case appropriately.
396-410: LGTM - Proper transactional winner selection.Good use of
$transactionto atomically update both the A/B test status and mark the winning variant, ensuring data consistency.apps/web/src/app/(dashboard)/sequences/[sequenceId]/page.tsx (4)
1-68: LGTM - Well-organized imports and constants.Imports follow the coding guidelines with
~/alias usage. TheSTATUS_COLORSandSTEP_ICONSconstants provide a clean mapping for UI rendering.
251-287: LGTM - Clean stats rendering.The conditional
{stats && (...)}properly guards against undefined stats. The grid layout is responsive with appropriate breakpoints.
318-421: LGTM - Well-structured step builder UI.The step rendering logic handles all step types appropriately with type-specific displays. The insertion controls between steps provide good UX for building sequences. The
canEditguard is consistently applied to all editing controls.
441-446: No issue here –contactBookIdis correctly typed and passed.The
EnrollContactsDialogcomponent explicitly acceptscontactBookId: string | null(line 29 of enroll-contacts-dialog.tsx), which matches the Prisma schema wherecontactBookIdis defined asString?(optional). The component properly handles null values by defaulting to an empty string when needed (line 43). The type ofsequence.contactBookIdfrom the API matches the prop type exactly, so there is no TypeScript mismatch.apps/web/src/server/api/routers/sequence.ts (3)
1-14: LGTM - Clean imports.Imports follow coding guidelines with proper enum imports from Prisma and TRPC utilities.
15-63: LGTM - Well-structured list procedure.Proper pagination, team-scoped filtering, and efficient use of
Promise.allfor parallel queries. Including_countfor steps and enrollments provides useful summary data.
679-749: LGTM - Comprehensive stats aggregation.The stats procedure provides well-structured data for the UI, with proper handling of zero-division cases and clean aggregation of enrollment counts and email metrics.
| conditionType String? // "opened", "clicked", "not_opened", "not_clicked" | ||
| conditionValue String? // e.g., specific link URL for click condition | ||
| yesStepId String? // Step to go to if condition is true | ||
| noStepId String? // Step to go to if condition is false |
There was a problem hiding this comment.
yesStepId and noStepId lack foreign key relations.
These fields reference other SequenceStep records but aren't defined as relations. This allows invalid step IDs to be stored and prevents cascading updates/deletes. While self-referential FKs can be tricky, consider adding relations or at least application-level validation.
Example relation addition (optional)
// For CONDITION type
conditionType String? // "opened", "clicked", "not_opened", "not_clicked"
conditionValue String? // e.g., specific link URL for click condition
yesStepId String? // Step to go to if condition is true
noStepId String? // Step to go to if condition is false
+
+ yesStep SequenceStep? @relation("YesBranch", fields: [yesStepId], references: [id], onDelete: SetNull)
+ noStep SequenceStep? @relation("NoBranch", fields: [noStepId], references: [id], onDelete: SetNull)Note: Self-referential relations require named relations to avoid ambiguity.
📝 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.
| conditionType String? // "opened", "clicked", "not_opened", "not_clicked" | |
| conditionValue String? // e.g., specific link URL for click condition | |
| yesStepId String? // Step to go to if condition is true | |
| noStepId String? // Step to go to if condition is false | |
| conditionType String? // "opened", "clicked", "not_opened", "not_clicked" | |
| conditionValue String? // e.g., specific link URL for click condition | |
| yesStepId String? // Step to go to if condition is true | |
| noStepId String? // Step to go to if condition is false | |
| yesStep SequenceStep? @relation("YesBranch", fields: [yesStepId], references: [id], onDelete: SetNull) | |
| noStep SequenceStep? @relation("NoBranch", fields: [noStepId], references: [id], onDelete: SetNull) |
🤖 Prompt for AI Agents
In @apps/web/prisma/schema.prisma around lines 748 - 751, The schema fields
yesStepId and noStepId are plain strings referencing other SequenceStep records
but lack foreign-key relations; add explicit self-referential relations on the
SequenceStep model (e.g., fields yesStepId and noStepId paired with relation
fields yesStep and noStep using named relations like SequenceStep_yes and
SequenceStep_no) or, if you prefer not to change DB schema, implement
application-level validation in the SequenceStep create/update logic to verify
referenced step IDs exist and enforce cascade/constraint behavior; after adding
relations, set appropriate referential actions (onDelete/onUpdate) and run
prisma migrate to apply the change and update any code that reads/writes
yesStepId/noStepId to use the new relation fields (SequenceStep.yesStep,
SequenceStep.noStep).
| const handleAdd = () => { | ||
| const baseData = { | ||
| sequenceId, | ||
| type: stepType, | ||
| name: name.trim() || undefined, | ||
| afterStepId, | ||
| }; | ||
|
|
||
| if (stepType === "EMAIL") { | ||
| if (!subject.trim() && !templateId) { | ||
| toast.error("Please enter a subject line or select a template"); | ||
| return; | ||
| } | ||
| addStepMutation.mutate({ | ||
| ...baseData, | ||
| subject: subject.trim() || undefined, | ||
| previewText: previewText.trim() || undefined, | ||
| html: html.trim() || undefined, | ||
| templateId, | ||
| }); | ||
| } else if (stepType === "DELAY") { | ||
| addStepMutation.mutate({ | ||
| ...baseData, | ||
| delayDuration, | ||
| delayUnit, | ||
| }); | ||
| } else if (stepType === "CONDITION") { | ||
| addStepMutation.mutate({ | ||
| ...baseData, | ||
| conditionType, | ||
| }); | ||
| } else if (stepType === "GOAL") { | ||
| addStepMutation.mutate({ | ||
| ...baseData, | ||
| goalType, | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the sequence router for delayDuration validation
rg -n "delayDuration" apps/web/src/server/api/routers/sequence.ts -A 5 -B 2Repository: usesend/useSend
Length of output: 853
🏁 Script executed:
# Search for step mutation or addStep related validation schemas
rg -n "addStep|createStep" apps/web/src/server/api/routers/sequence.ts -A 10Repository: usesend/useSend
Length of output: 464
🏁 Script executed:
# Look for any Zod schemas related to step creation
fd -e ts -e tsx apps/web/src/server | xargs rg -l "delayDuration" | head -10Repository: usesend/useSend
Length of output: 692
🏁 Script executed:
# Get the full addStep schema and handler
sed -n '284,330p' apps/web/src/server/api/routers/sequence.tsRepository: usesend/useSend
Length of output: 1565
🏁 Script executed:
# Check if there's validation logic inside the handler
sed -n '284,400p' apps/web/src/server/api/routers/sequence.ts | grep -A 50 "addStep:"Repository: usesend/useSend
Length of output: 1738
🏁 Script executed:
# Get more of the handler to see the full logic
sed -n '284,450p' apps/web/src/server/api/routers/sequence.tsRepository: usesend/useSend
Length of output: 4984
🏁 Script executed:
# Check the database schema to see if there are constraints at DB level
rg -n "delayDuration" apps/web/prisma/schema.prisma -B 5 -A 5Repository: usesend/useSend
Length of output: 577
Add Zod validation to require delayDuration >= 1 for DELAY steps.
The Zod schema defines delayDuration as a basic z.number().optional() without minimum value constraints. The mutation handler also lacks type-specific validation—it directly creates/updates the database record without checking if delayDuration >= 1 when type === "DELAY". A user could submit zero, negative, or invalid delay values via browser dev tools. Update the Zod schema to use .min(1) for delayDuration and optionally add conditional validation in the handler to ensure delays are sensible for DELAY type steps.
🤖 Prompt for AI Agents
In @apps/web/src/app/(dashboard)/sequences/[sequenceId]/add-step-dialog.tsx
around lines 91 - 128, Update the Zod schema that defines delayDuration to
require a minimum of 1 (e.g., change delayDuration from z.number().optional() to
z.number().min(1).optional() or the equivalent in your schema) and add a runtime
guard in the add handler (handleAdd) to enforce this for DELAY steps: when
stepType === "DELAY" check that delayDuration is a number >= 1, show a toast
error and return if it isn’t, otherwise call addStepMutation.mutate with the
delay fields; this ensures both schema-level and handler-level protection
against zero/negative delays.
| const contactBooksQuery = api.contacts.getContactBooks.useQuery(); | ||
|
|
||
| const contactsQuery = api.contacts.getContacts.useQuery( | ||
| { | ||
| contactBookId: selectedContactBookId, | ||
| page: 1, | ||
| search: searchQuery || undefined, | ||
| }, | ||
| { | ||
| enabled: !!selectedContactBookId, | ||
| } | ||
| ); |
There was a problem hiding this comment.
"Select all" only selects contacts from the first page.
The contacts query only fetches page: 1, so the "Select all" feature will only select visible contacts. Users might expect it to select all contacts matching the search, not just the first page.
Consider either:
- Documenting this behavior in the UI (e.g., "Select all visible")
- Implementing server-side bulk enrollment with filters
- Fetching all contact IDs for the "select all" operation
♻️ Quick UI fix
- <span className="text-sm font-medium">
- Select all ({contactsQuery.data?.contacts.length || 0}{" "}
- contacts)
- </span>
+ <span className="text-sm font-medium">
+ Select all visible ({contactsQuery.data?.contacts.length || 0}{" "}
+ contacts)
+ </span>| updateMutation.mutate({ | ||
| id: sequence.id, | ||
| name: name.trim(), | ||
| description: description.trim() || undefined, | ||
| triggerType, | ||
| contactBookId: contactBookId || null, | ||
| fromEmail: fromEmail.trim() || undefined, | ||
| fromName: fromName.trim() || undefined, | ||
| replyTo: replyTo.trim() || undefined, | ||
| exitOnUnsubscribe, | ||
| exitOnGoal, | ||
| allowReentry, | ||
| }); |
There was a problem hiding this comment.
Inconsistent handling of optional fields.
Some fields use || undefined (lines 105, 108-110) while contactBookId uses || null (line 107). This inconsistency could cause issues depending on how the backend schema handles these values.
🤖 Prompt for AI Agents
In
@apps/web/src/app/(dashboard)/sequences/[sequenceId]/sequence-settings-dialog.tsx
around lines 102 - 114, The updateMutation payload mixes undefined and null for
optional fields causing inconsistency; pick one representation expected by the
backend and apply it uniformly in the updateMutation.mutate call: change
contactBookId to use the same pattern as description/fromEmail/fromName/replyTo
(either `|| undefined` or `|| null`) and ensure any trimming (e.g., name.trim(),
fromEmail.trim()) remains; update the fields in the mutate call (id, name,
description, triggerType, contactBookId, fromEmail, fromName, replyTo,
exitOnUnsubscribe, exitOnGoal, allowReentry) to use the chosen consistent
optional value handling.
| <div className="space-y-2"> | ||
| <label className="text-sm font-medium">Name</label> | ||
| <Input | ||
| value={name} | ||
| onChange={(e) => setName(e.target.value)} | ||
| placeholder="Welcome Series" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Labels are not properly associated with inputs for accessibility.
The <label> elements use class styling but lack htmlFor attributes linking them to input ids. This impacts screen reader users and reduces click target area.
Example fix for one field
<div className="space-y-2">
- <label className="text-sm font-medium">Name</label>
+ <label htmlFor="sequence-name" className="text-sm font-medium">Name</label>
<Input
+ id="sequence-name"
value={name}
onChange={(e) => setName(e.target.value)}
placeholder="Welcome Series"
/>
</div>📝 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.
| <div className="space-y-2"> | |
| <label className="text-sm font-medium">Name</label> | |
| <Input | |
| value={name} | |
| onChange={(e) => setName(e.target.value)} | |
| placeholder="Welcome Series" | |
| /> | |
| </div> | |
| <div className="space-y-2"> | |
| <label htmlFor="sequence-name" className="text-sm font-medium">Name</label> | |
| <Input | |
| id="sequence-name" | |
| value={name} | |
| onChange={(e) => setName(e.target.value)} | |
| placeholder="Welcome Series" | |
| /> | |
| </div> |
🤖 Prompt for AI Agents
In
@apps/web/src/app/(dashboard)/sequences/[sequenceId]/sequence-settings-dialog.tsx
around lines 138 - 145, The label for the Name field is not associated with its
input; add an id on the Input component (e.g., id="sequence-name") and set the
label's htmlFor to that id so screen readers and click-to-focus work; update the
Input in the sequence-settings-dialog component (the Input with value={name} and
onChange={(e) => setName(e.target.value)}) and ensure any other label/Input
pairs in this dialog follow the same pattern.
| if (input.format === "csv") { | ||
| const headers = [ | ||
| "Timestamp", | ||
| "User", | ||
| "Action", | ||
| "Resource Type", | ||
| "Resource Name", | ||
| "Details", | ||
| ]; | ||
| const rows = logs.map((log) => [ | ||
| log.createdAt.toISOString(), | ||
| log.user?.email || "System", | ||
| log.action, | ||
| log.resourceType, | ||
| log.resourceName || "", | ||
| JSON.stringify(log.details || {}), | ||
| ]); | ||
|
|
||
| const csv = [headers, ...rows].map((row) => row.join(",")).join("\n"); | ||
| return { data: csv, format: "csv" as const }; |
There was a problem hiding this comment.
CSV output lacks proper escaping, which can corrupt data.
The CSV generation joins fields with commas without escaping values that contain commas, quotes, or newlines. If resourceName or details contains a comma, the CSV will be malformed.
Fix with proper CSV escaping
+ const escapeCsvField = (field: string): string => {
+ if (field.includes(',') || field.includes('"') || field.includes('\n')) {
+ return `"${field.replace(/"/g, '""')}"`;
+ }
+ return field;
+ };
+
if (input.format === "csv") {
const headers = [
"Timestamp",
"User",
"Action",
"Resource Type",
"Resource Name",
"Details",
];
const rows = logs.map((log) => [
- log.createdAt.toISOString(),
- log.user?.email || "System",
- log.action,
- log.resourceType,
- log.resourceName || "",
- JSON.stringify(log.details || {}),
+ escapeCsvField(log.createdAt.toISOString()),
+ escapeCsvField(log.user?.email || "System"),
+ escapeCsvField(log.action),
+ escapeCsvField(log.resourceType),
+ escapeCsvField(log.resourceName || ""),
+ escapeCsvField(JSON.stringify(log.details || {})),
]);
- const csv = [headers, ...rows].map((row) => row.join(",")).join("\n");
+ const csv = [headers.map(escapeCsvField), ...rows].map((row) => row.join(",")).join("\n");
return { data: csv, format: "csv" as const };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (input.format === "csv") { | |
| const headers = [ | |
| "Timestamp", | |
| "User", | |
| "Action", | |
| "Resource Type", | |
| "Resource Name", | |
| "Details", | |
| ]; | |
| const rows = logs.map((log) => [ | |
| log.createdAt.toISOString(), | |
| log.user?.email || "System", | |
| log.action, | |
| log.resourceType, | |
| log.resourceName || "", | |
| JSON.stringify(log.details || {}), | |
| ]); | |
| const csv = [headers, ...rows].map((row) => row.join(",")).join("\n"); | |
| return { data: csv, format: "csv" as const }; | |
| const escapeCsvField = (field: string): string => { | |
| if (field.includes(',') || field.includes('"') || field.includes('\n')) { | |
| return `"${field.replace(/"/g, '""')}"`; | |
| } | |
| return field; | |
| }; | |
| if (input.format === "csv") { | |
| const headers = [ | |
| "Timestamp", | |
| "User", | |
| "Action", | |
| "Resource Type", | |
| "Resource Name", | |
| "Details", | |
| ]; | |
| const rows = logs.map((log) => [ | |
| escapeCsvField(log.createdAt.toISOString()), | |
| escapeCsvField(log.user?.email || "System"), | |
| escapeCsvField(log.action), | |
| escapeCsvField(log.resourceType), | |
| escapeCsvField(log.resourceName || ""), | |
| escapeCsvField(JSON.stringify(log.details || {})), | |
| ]); | |
| const csv = [headers.map(escapeCsvField), ...rows].map((row) => row.join(",")).join("\n"); | |
| return { data: csv, format: "csv" as const }; | |
| } |
| await db.contactEngagementPattern.update({ | ||
| where: { contactId: input.contactId }, | ||
| data: { | ||
| hourlyScores, | ||
| dayOfWeekScores, | ||
| bestHourUtc: bestHour, | ||
| bestDayOfWeek: bestDay, | ||
| totalOpens: | ||
| input.eventType === "open" | ||
| ? { increment: 1 } | ||
| : existingPattern.totalOpens, | ||
| totalClicks: | ||
| input.eventType === "click" | ||
| ? { increment: 1 } | ||
| : existingPattern.totalClicks, | ||
| lastCalculatedAt: new Date(), | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Inconsistent update pattern for totalOpens/totalClicks could cause race conditions.
The code uses { increment: 1 } for the matching event type but the raw value for the non-matching type. If concurrent updates occur, the raw value assignment could overwrite increments from other transactions.
🔧 Consistent increment pattern
await db.contactEngagementPattern.update({
where: { contactId: input.contactId },
data: {
hourlyScores,
dayOfWeekScores,
bestHourUtc: bestHour,
bestDayOfWeek: bestDay,
- totalOpens:
- input.eventType === "open"
- ? { increment: 1 }
- : existingPattern.totalOpens,
- totalClicks:
- input.eventType === "click"
- ? { increment: 1 }
- : existingPattern.totalClicks,
+ ...(input.eventType === "open" && { totalOpens: { increment: 1 } }),
+ ...(input.eventType === "click" && { totalClicks: { increment: 1 } }),
lastCalculatedAt: new Date(),
},
});🤖 Prompt for AI Agents
In @apps/web/src/server/api/routers/send-time.ts around lines 307 - 324, The
update to db.contactEngagementPattern.update currently assigns raw values for
non-matching counters which can overwrite concurrent increments; change
totalOpens and totalClicks to use atomic increments instead (e.g., totalOpens: {
increment: input.eventType === "open" ? 1 : 0 } and totalClicks: { increment:
input.eventType === "click" ? 1 : 0 }) so both branches use Prisma's increment
operation rather than writing
existingPattern.totalOpens/existingPattern.totalClicks.
| // Get all contacts in the contact book | ||
| const contacts = await db.contact.findMany({ | ||
| where: { | ||
| contactBookId: input.contactBookId, | ||
| contactBook: { teamId: team.id }, | ||
| }, | ||
| select: { id: true }, | ||
| }); | ||
|
|
||
| const contactIds = contacts.map((c) => c.id); |
There was a problem hiding this comment.
Unbounded query could cause performance issues for large contact books.
The findMany query has no limit, which could retrieve hundreds of thousands of contacts for large books, causing memory and performance issues.
🔧 Add a reasonable limit or pagination
const contacts = await db.contact.findMany({
where: {
contactBookId: input.contactBookId,
contactBook: { teamId: team.id },
},
select: { id: true },
+ take: 50000, // Reasonable upper limit for aggregation
});Alternatively, consider aggregating engagement patterns directly without fetching contact IDs first, or sampling a representative subset for very large books.
📝 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.
| // Get all contacts in the contact book | |
| const contacts = await db.contact.findMany({ | |
| where: { | |
| contactBookId: input.contactBookId, | |
| contactBook: { teamId: team.id }, | |
| }, | |
| select: { id: true }, | |
| }); | |
| const contactIds = contacts.map((c) => c.id); | |
| // Get all contacts in the contact book | |
| const contacts = await db.contact.findMany({ | |
| where: { | |
| contactBookId: input.contactBookId, | |
| contactBook: { teamId: team.id }, | |
| }, | |
| select: { id: true }, | |
| take: 50000, // Reasonable upper limit for aggregation | |
| }); | |
| const contactIds = contacts.map((c) => c.id); |
🤖 Prompt for AI Agents
In @apps/web/src/server/api/routers/send-time.ts around lines 363 - 372, The
findMany call retrieving all contacts (db.contact.findMany used to build
contacts -> contactIds for input.contactBookId and team.id) is unbounded and can
OOM on large contact books; change the logic to use pagination or a size cap:
either paginate through results with cursor/limit and process in batches (using
limit + skip or cursor-based queries) to accumulate or stream contactIds, or add
a configurable limit/sample when full processing isn't required; alternatively
perform aggregation on the DB side (avoid fetching all ids) if you only need
counts/patterns. Ensure updates touch the code that consumes contactIds so it
can handle batched/streamed arrays instead of assuming a single full array.
| await Promise.all( | ||
| input.stepIds.map((stepId, idx) => | ||
| db.sequenceStep.update({ | ||
| where: { id: stepId }, | ||
| data: { order: idx }, | ||
| }) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Missing validation that step IDs belong to the sequence.
The procedure updates step orders based on provided stepIds without verifying they belong to input.sequenceId. A malicious actor could potentially reorder steps in other sequences if they know the step IDs.
🔒 Add step ownership validation
if (sequence.status === "ACTIVE") {
throw new TRPCError({
code: "BAD_REQUEST",
message: "Cannot reorder steps in an active sequence",
});
}
+ // Verify all steps belong to this sequence
+ const steps = await db.sequenceStep.findMany({
+ where: { id: { in: input.stepIds }, sequenceId: input.sequenceId },
+ select: { id: true },
+ });
+
+ if (steps.length !== input.stepIds.length) {
+ throw new TRPCError({
+ code: "BAD_REQUEST",
+ message: "One or more steps do not belong to this sequence",
+ });
+ }
await Promise.all(
input.stepIds.map((stepId, idx) =>
db.sequenceStep.update({
where: { id: stepId },
data: { order: idx },
})
)
);🤖 Prompt for AI Agents
In @apps/web/src/server/api/routers/sequence.ts around lines 486 - 493, The
update blindly reorders steps using the provided input.stepIds without
confirming ownership; fetch the sequenceStep records for input.stepIds (e.g.,
via db.sequenceStep.findMany with id in input.stepIds), verify the returned set
length equals input.stepIds.length and that every record has sequenceId ===
input.sequenceId, and if any mismatch throw an authorization/bad-request error;
only after this validation proceed with the Promise.all of
db.sequenceStep.update calls (optionally wrap in a transaction) so you can't
reorder steps from other sequences.
| await Promise.all( | ||
| webhooks.map(async (webhook) => { | ||
| const delivery = await db.webhookDelivery.create({ | ||
| data: { | ||
| webhookId: webhook.id, | ||
| emailId, | ||
| eventType: webhookEvent, | ||
| payload: payload as any, | ||
| status: WebhookDeliveryStatus.PENDING, | ||
| }, | ||
| }); | ||
|
|
||
| await this.webhookQueue.add( | ||
| delivery.id, | ||
| { | ||
| webhookId: webhook.id, | ||
| deliveryId: delivery.id, | ||
| payload, | ||
| }, | ||
| { | ||
| ...DEFAULT_QUEUE_OPTIONS, | ||
| attempts: MAX_RETRY_ATTEMPTS, | ||
| backoff: { | ||
| type: "exponential", | ||
| delay: 5000, // Start with 5 seconds, then 10, 20... | ||
| }, | ||
| } | ||
| ); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Partial failure leaves orphaned delivery records.
If Promise.all fails partway through (e.g., queue connection issue on the 3rd webhook), earlier delivery records are created but their jobs may not be queued, leaving them stuck in PENDING status indefinitely.
Consider using Promise.allSettled and handling partial failures, or wrapping each iteration in a try-catch to ensure all webhooks get attempted.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/src/server/api/routers/sequence.ts">
<violation number="1" location="apps/web/src/server/api/routers/sequence.ts:450">
P0: Critical bug: Uses wrong variable `stepId` (the deleted step) instead of `s.id` (each remaining step). Also, `input.sequenceId` doesn't exist in this mutation's input schema (only `stepId` is defined), so it will be undefined. This will cause the reordering of remaining steps to fail.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| await Promise.all( | ||
| remainingSteps.map((s, idx) => | ||
| db.sequenceStep.update({ | ||
| where: { id: stepId, sequenceId: input.sequenceId }, |
There was a problem hiding this comment.
P0: Critical bug: Uses wrong variable stepId (the deleted step) instead of s.id (each remaining step). Also, input.sequenceId doesn't exist in this mutation's input schema (only stepId is defined), so it will be undefined. This will cause the reordering of remaining steps to fail.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/server/api/routers/sequence.ts, line 450:
<comment>Critical bug: Uses wrong variable `stepId` (the deleted step) instead of `s.id` (each remaining step). Also, `input.sequenceId` doesn't exist in this mutation's input schema (only `stepId` is defined), so it will be undefined. This will cause the reordering of remaining steps to fail.</comment>
<file context>
@@ -447,9 +447,10 @@ export const sequenceRouter = createTRPCRouter({
remainingSteps.map((s, idx) =>
db.sequenceStep.update({
- where: { id: s.id },
+ where: { id: stepId, sequenceId: input.sequenceId },
data: { order: idx },
})
</file context>
| where: { id: stepId, sequenceId: input.sequenceId }, | |
| where: { id: s.id }, |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/src/server/service/email-service.ts">
<violation number="1" location="apps/web/src/server/service/email-service.ts:382">
P2: Error message is now inconsistent with allowed statuses. The message still mentions 'complained' emails can be resent, but `COMPLAINED` was removed from `allowedStatuses`. Update the message to match: `"Only failed, bounced, or rejected emails can be resent."`</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| // Only allow resending failed or bounced emails | ||
| const allowedStatuses = ["FAILED", "BOUNCED", "REJECTED"]; |
There was a problem hiding this comment.
P2: Error message is now inconsistent with allowed statuses. The message still mentions 'complained' emails can be resent, but COMPLAINED was removed from allowedStatuses. Update the message to match: "Only failed, bounced, or rejected emails can be resent."
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/server/service/email-service.ts, line 382:
<comment>Error message is now inconsistent with allowed statuses. The message still mentions 'complained' emails can be resent, but `COMPLAINED` was removed from `allowedStatuses`. Update the message to match: `"Only failed, bounced, or rejected emails can be resent."`</comment>
<file context>
@@ -379,7 +379,7 @@ export async function resendEmail(emailId: string, teamId: number) {
// Only allow resending failed or bounced emails
- const allowedStatuses = ["FAILED", "BOUNCED", "REJECTED", "COMPLAINED"];
+ const allowedStatuses = ["FAILED", "BOUNCED", "REJECTED"];
if (!allowedStatuses.includes(originalEmail.latestStatus)) {
throw new UnsendApiError({
</file context>
|
i'm not gonna merge 77 files changes with 11k additions that's completely written by ai. sorry! break the problems, make small prs that i can review |
2 participants
Summary
This PR adds comprehensive enhancements across three tiers of features:
Tier 1 (Quick Wins)
Tier 2 (Medium Effort)
Tier 3 (Higher Effort)
Technical Changes
ScheduledReport,Segment,Webhook,WebhookDeliverysegment,scheduled-report,webhook,settingsemail(search/resend),campaign(comparison),contacts(duplicates)webhook-service.tsfor delivery handling@radix-ui/react-alert-dialogdependencyNew Pages/Routes
/dev-settings/webhooks- Webhook management/dev-settings/usage- API usage dashboard/contacts/[id]/segments- Contact segmentation/contacts/duplicates- Duplicate detection/settings/reports- Scheduled reports/campaigns/compare- Campaign comparisonTest Plan
🤖 Generated with Claude Code
Summary by cubic
Adds Tier 1–4 upgrades across webhooks, segmentation, reporting, analytics, automation, and email UX. Adds A/B testing, sequences, send‑time optimization, audit logs, stronger security, and several quality‑of‑life improvements.
New Features
Technical Changes
Written for commit 4388019. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Dependencies
✏️ Tip: You can customize this high-level summary in your review settings.