Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds team verification/blocking and daily email limits; introduces Redis read-through caching and team cache helpers; implements cloud-gated email limit checks with daily/monthly enforcement and notifications; integrates pre-send limit checks into the email queue; adds warning/limit email templates and related service/router/migration changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Worker
participant EQS as EmailQueueService
participant LS as LimitService
participant TS as TeamService
participant RD as Redis
participant DB as DB
participant SES as Sender
participant ML as Mailer
Worker->>EQS: processQueuedEmail(email)
EQS->>LS: checkEmailLimit(teamId)
LS->>TS: getTeamCached(teamId)
TS->>RD: GET team:{id}
alt Cache hit
RD-->>TS: Team JSON
else Cache miss
TS->>DB: findUnique(team)
DB-->>TS: Team
TS->>RD: SETEX team:{id}
end
TS-->>LS: Team
LS->>DB: count usage (daily/monthly)
LS-->>EQS: {isLimitReached, limit, reason, available?}
alt Limit reached
EQS->>DB: record FAILED event, update status
EQS->>TS: maybeNotifyEmailLimitReached(teamId, limit, reason)
TS->>RD: INCR notify:{team}:{reason}
alt not recently notified
TS->>ML: sendMail(limit-reached email)
else throttled
TS-->>TS: skip send
end
EQS-->>Worker: abort send
else Allowed
EQS->>SES: send(email)
alt success
EQS-->>Worker: done
else failure
EQS->>DB: mark FAILED
end
end
sequenceDiagram
autonumber
participant Svc as Service
participant TS as TeamService
participant RD as Redis
participant DB as DB
Svc->>TS: getTeamCached(teamId)
TS->>RD: GET team:{id}
alt hit
RD-->>TS: Team JSON
TS-->>Svc: Team
else miss
TS->>DB: findUnique(team)
DB-->>TS: Team
TS->>RD: SETEX team:{id}, 120s
TS-->>Svc: Team
end
Svc->>TS: updateTeam(teamId, data)
TS->>DB: update(team)
DB-->>TS: Team
TS->>RD: SETEX team:{id}, 120s
TS-->>Svc: Team
sequenceDiagram
autonumber
participant TS as TeamService
participant RD as Redis
participant ML as Mailer
TS->>RD: INCR notify:{team}:{type}
alt first notify
RD-->>TS: 1
TS->>RD: EXPIRE notify:{team}:{type}, 6h
TS->>ML: sendMail(recipients, subject, html)
else throttled
RD-->>TS: >1
TS-->>TS: skip sending
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/web/src/server/service/email-service.ts (2)
218-222: Validate scheduledAt to prevent NaN delays reaching the queueInvalid dates yield NaN, which may propagate to the queue API.
- const scheduledAtDate = scheduledAt ? new Date(scheduledAt) : undefined; - const delay = scheduledAtDate - ? Math.max(0, scheduledAtDate.getTime() - Date.now()) - : undefined; + const scheduledAtDate = scheduledAt ? new Date(scheduledAt) : undefined; + if (scheduledAt && (!scheduledAtDate || Number.isNaN(scheduledAtDate.getTime()))) { + throw new UnsendApiError({ code: "BAD_REQUEST", message: "Invalid scheduledAt value" }); + } + const delay = scheduledAtDate + ? Math.max(0, scheduledAtDate.getTime() - Date.now()) + : undefined;Also applies to: 247-255
294-306: Same scheduledAt validation needed on updateEmail- const scheduledAtDate = scheduledAt ? new Date(scheduledAt) : undefined; + const scheduledAtDate = scheduledAt ? new Date(scheduledAt) : undefined; + if (scheduledAt && (!scheduledAtDate || Number.isNaN(scheduledAtDate.getTime()))) { + throw new UnsendApiError({ code: "BAD_REQUEST", message: "Invalid scheduledAt value" }); + } const delay = scheduledAtDate ? Math.max(0, scheduledAtDate.getTime() - Date.now()) : undefined;apps/web/src/server/mailer.ts (1)
72-77: Export looks good; however env check will skip USESEND_API_KEY and misreport missing configBranch uses only UNSEND_API_KEY, but getClient supports USESEND_API_KEY too. Also error message omits FROM_EMAIL.
-} else if (env.UNSEND_API_KEY && env.FROM_EMAIL) { +} else if ((env.USESEND_API_KEY || env.UNSEND_API_KEY) && env.FROM_EMAIL) { const resp = await getClient().emails.send({ @@ - } else { - throw new Error("USESEND_API_KEY/UNSEND_API_KEY not found"); + } else { + throw new Error("USESEND_API_KEY or UNSEND_API_KEY (and FROM_EMAIL) not configured"); }apps/web/src/server/redis.ts (1)
8-11: Fix Redis URL query handling (breaks when REDIS_URL already has query).String-appending
?family=0can yield invalid URLs (e.g., duplicate?). Use URLSearchParams.- connection = new IORedis(`${env.REDIS_URL}?family=0`, { + const url = new URL(env.REDIS_URL); + if (!url.searchParams.has("family")) url.searchParams.set("family", "0"); + connection = new IORedis(url.toString(), { maxRetriesPerRequest: null, });apps/web/src/server/api/routers/limits.ts (1)
21-24: Use TRPCError for bad requests.Surface a 400 instead of throwing a generic error.
+import { TRPCError } from "@trpc/server"; ... - throw new Error("Unsupported limit type"); + throw new TRPCError({ code: "BAD_REQUEST", message: "Unsupported limit type" });
🧹 Nitpick comments (19)
AGENTS.md (1)
30-30: Scope “Never use dynamic imports” or it may block legitimate Next.js patternsDynamic imports are occasionally required (e.g., client-only libs, heavy editor components). Suggest softening and scoping the rule to apps/web with explicit exceptions and PR justification.
-- Never use dynamic imports +- Avoid dynamic imports. Prefer static imports. Use Next.js dynamic() only for client-only or heavy, non-SSR-safe components with a brief justification in the PR.apps/web/src/server/service/domain-service.ts (2)
90-95: Wrap SES domain creation with contextual error handlingIf ses.addDomain throws (network/AWS error), callers get a generic Error. Consider wrapping with UnsendApiError plus logging to aid debugging.
- const publicKey = await ses.addDomain( + let publicKey: string; + try { + publicKey = await ses.addDomain( name, region, sesTenantId, dkimSelector - ); + ); + } catch (err) { + logger.error({ err, name, region }, "SES addDomain failed"); + throw new UnsendApiError({ code: "INTERNAL_SERVER_ERROR", message: "Failed to set up domain with SES" }); + }
198-201: Name nit: return the deleted domain with a clearer identifierMinor readability tweak.
- const deletedRecord = await db.domain.delete({ where: { id } }); - - return deletedRecord; + const deletedDomain = await db.domain.delete({ where: { id } }); + return deletedDomain;apps/web/src/server/api/routers/invitiation.ts (1)
73-73: Clarify which cache this refers toAdd a short pointer (e.g., “TeamService cache”) so future readers know why invalidation isn’t needed here.
- // No need to invalidate cache here again + // No need to invalidate TeamService cache here again (invite deletion does not affect team cache)apps/web/src/server/service/email-service.ts (1)
36-45: Escape variable keys in RegExp to avoid accidental regex meta interpretationKeys containing regex chars (., +, etc.) can mis-match. Escape before building the pattern.
- return Object.keys(variables).reduce((accum, key) => { - const re = new RegExp(`{{${key}}}`, "g"); + return Object.keys(variables).reduce((accum, key) => { + const escapedKey = key.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const re = new RegExp(`{{${escapedKey}}}`, "g"); const returnTxt = accum.replace(re, variables[key] as string); return returnTxt; }, text);apps/web/src/server/mailer.ts (1)
120-128: Log message promises a resend fallback that doesn’t existEither implement a fallback or update the log to reflect no retry.
- "Error sending email using usesend, so fallback to resend" + "Error sending email using usesend; no fallback configured"apps/web/src/server/redis.ts (1)
41-47: Defer Redis connection when caching is disabled.Avoid creating a connection when
disableis true.- if (!disable) { + if (!disable) { + const redis = getRedis(); try { - await redis.setex(key, ttlSeconds, JSON.stringify(value)); + await redis.setex(key, ttlSeconds, JSON.stringify(value)); } catch { // ignore cache set errors } }apps/web/src/server/email-templates/UsageLimitReachedEmail.tsx (2)
76-78: Copy tweak for clarity.Consider ending the sentence with a period and verifying the conditional phrasing aligns with your policy (paid plans → “verified”; free → “upgraded”).
81-83: Hide CTA when no manage URL is provided.Avoid
href="#"in production emails.- manageUrl = "#", + manageUrl,- <Container style={{ margin: "0 0 32px 0", textAlign: "left" as const }}> - <EmailButton href={manageUrl}>Manage plan</EmailButton> - </Container> + {manageUrl && ( + <Container style={{ margin: "0 0 32px 0", textAlign: "left" as const }}> + <EmailButton href={manageUrl}>Manage plan</EmailButton> + </Container> + )}apps/web/src/server/api/routers/billing.ts (1)
51-52: Centralizing writes via TeamService looks good.This keeps cache coherence in one place. Optionally return the updated team or a boolean for client UX.
apps/web/src/server/service/email-queue-service.ts (2)
13-14: Use app alias imports in apps/webPrefer "~/" alias over relative paths per repo guidelines.
-import { LimitService } from "./limit-service"; -import { TeamService } from "./team-service"; +import { LimitService } from "~/server/service/limit-service"; +import { TeamService } from "~/server/service/team-service";
371-402: Pre-send limit block: make event+status update atomicWrap the two writes in a single transaction to avoid partial updates.
- await db.emailEvent.create({ - data: { - emailId: email.id, - status: "FAILED", - data: { - error: "Email sending limit reached", - reason: limitCheck.reason, - limit: limitCheck.limit, - }, - teamId: email.teamId, - }, - }); - await db.email.update({ - where: { id: email.id }, - data: { latestStatus: "FAILED" }, - }); + await db.$transaction([ + db.emailEvent.create({ + data: { + emailId: email.id, + status: "FAILED", + data: { + error: "Email sending limit reached", + reason: limitCheck.reason, + limit: limitCheck.limit, + }, + teamId: email.teamId, + }, + }), + db.email.update({ + where: { id: email.id }, + data: { latestStatus: "FAILED" }, + }), + ]);apps/web/src/app/api/dev/email-preview/route.ts (1)
33-55: Harden “period” parsing to allowed valuesAvoid passing arbitrary strings by normalizing to "daily" | "monthly".
- } else if (type === "usage-warning") { - const isPaidPlan = searchParams.get("isPaidPlan") === "true"; - const period = searchParams.get("period") || "daily"; + } else if (type === "usage-warning") { + const isPaidPlan = searchParams.get("isPaidPlan") === "true"; + const rawPeriod = searchParams.get("period"); + const period = rawPeriod === "monthly" ? "monthly" : "daily"; @@ - } else if (type === "usage-limit") { - const isPaidPlan = searchParams.get("isPaidPlan") === "true"; - const period = searchParams.get("period") || "daily"; + } else if (type === "usage-limit") { + const isPaidPlan = searchParams.get("isPaidPlan") === "true"; + const rawPeriod = searchParams.get("period"); + const period = rawPeriod === "monthly" ? "monthly" : "daily";apps/web/src/server/email-templates/UsageWarningEmail.tsx (3)
4-7: Use "~/" alias for internal email componentsAligns with apps/web import conventions.
-import { EmailLayout } from "./components/EmailLayout"; -import { EmailHeader } from "./components/EmailHeader"; -import { EmailFooter } from "./components/EmailFooter"; -import { EmailButton } from "./components/EmailButton"; +import { EmailLayout } from "~/server/email-templates/components/EmailLayout"; +import { EmailHeader } from "~/server/email-templates/components/EmailHeader"; +import { EmailFooter } from "~/server/email-templates/components/EmailFooter"; +import { EmailButton } from "~/server/email-templates/components/EmailButton";
28-31: Clamp percent to 0–100Prevents odd values if used > limit or limit is tiny.
- const percent = limit > 0 ? Math.round((used / limit) * 100) : 80; + const rawPercent = limit > 0 ? Math.round((used / limit) * 100) : 80; + const percent = Math.max(0, Math.min(100, rawPercent));
58-61: Stabilize numeric formattingSpecify locale to avoid environment-dependent formatting in emails.
- <strong style={{ color: "#000" }}>{used.toLocaleString()}</strong> of + <strong style={{ color: "#000" }}>{used.toLocaleString("en-US")}</strong> of @@ - <strong style={{ color: "#000" }}>{limit.toLocaleString()}</strong>{" "} + <strong style={{ color: "#000" }}>{limit.toLocaleString("en-US")}</strong>{" "}apps/web/src/server/service/team-service.ts (1)
15-34: Cache TTL and serialization caveat120s TTL is fine. Note JSON.parse returns stringified dates; if any Team date fields are later used as Date objects, this will bite. Consider hydrating or caching only the fields you actually need for limits.
apps/web/src/server/service/limit-service.ts (2)
133-140: Clamp available to >= 0When exceeded, available is negative. Clamp to 0 for consistency in callers/UI.
Apply:
- available: dailyLimit - dailyUsage, + available: Math.max(0, dailyLimit - dailyUsage),
149-156: Warning threshold is good; avoid spammingGiven the email-side cooldown, this is fine. Optionally make 80% a constant.
- if (monthlyUsage / monthlyLimit > 0.8) { + const WARN_THRESHOLD = 0.8; + if (monthlyUsage / monthlyLimit > WARN_THRESHOLD) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
AGENTS.md(1 hunks)apps/web/prisma/migrations/20250907195449_add_team_verification_and_limits/migration.sql(1 hunks)apps/web/prisma/schema.prisma(1 hunks)apps/web/src/app/api/dev/email-preview/route.ts(2 hunks)apps/web/src/lib/constants/plans.ts(1 hunks)apps/web/src/server/api/routers/billing.ts(2 hunks)apps/web/src/server/api/routers/invitiation.ts(1 hunks)apps/web/src/server/api/routers/limits.ts(1 hunks)apps/web/src/server/billing/payments.ts(3 hunks)apps/web/src/server/email-templates/UsageLimitReachedEmail.tsx(1 hunks)apps/web/src/server/email-templates/UsageWarningEmail.tsx(1 hunks)apps/web/src/server/email-templates/components/EmailFooter.tsx(1 hunks)apps/web/src/server/email-templates/index.ts(1 hunks)apps/web/src/server/mailer.ts(1 hunks)apps/web/src/server/redis.ts(1 hunks)apps/web/src/server/service/contact-book-service.ts(2 hunks)apps/web/src/server/service/domain-service.ts(2 hunks)apps/web/src/server/service/email-queue-service.ts(3 hunks)apps/web/src/server/service/email-service.ts(17 hunks)apps/web/src/server/service/limit-service.ts(5 hunks)apps/web/src/server/service/team-service.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/lib/constants/plans.tsapps/web/src/server/mailer.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/billing/payments.tsapps/web/src/server/redis.tsapps/web/src/server/email-templates/components/EmailFooter.tsxapps/web/src/server/api/routers/billing.tsapps/web/src/server/api/routers/invitiation.tsapps/web/src/server/email-templates/UsageWarningEmail.tsxapps/web/src/server/email-templates/index.tsapps/web/src/server/service/contact-book-service.tsapps/web/src/server/service/email-queue-service.tsapps/web/src/server/email-templates/UsageLimitReachedEmail.tsxapps/web/src/server/api/routers/limits.tsapps/web/src/server/service/email-service.tsapps/web/src/server/service/limit-service.tsapps/web/src/app/api/dev/email-preview/route.tsapps/web/src/server/service/team-service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Prefer TypeScript for implementation files (TypeScript-first)
ESLint must pass with zero warnings using @usesend/eslint-config before PRs
Files:
apps/web/src/lib/constants/plans.tsapps/web/src/server/mailer.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/billing/payments.tsapps/web/src/server/redis.tsapps/web/src/server/email-templates/components/EmailFooter.tsxapps/web/src/server/api/routers/billing.tsapps/web/src/server/api/routers/invitiation.tsapps/web/src/server/email-templates/UsageWarningEmail.tsxapps/web/src/server/email-templates/index.tsapps/web/src/server/service/contact-book-service.tsapps/web/src/server/service/email-queue-service.tsapps/web/src/server/email-templates/UsageLimitReachedEmail.tsxapps/web/src/server/api/routers/limits.tsapps/web/src/server/service/email-service.tsapps/web/src/server/service/limit-service.tsapps/web/src/app/api/dev/email-preview/route.tsapps/web/src/server/service/team-service.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,md}: Use 2-space indentation and keep semicolons as enforced by Prettier 3
Run Prettier 3 formatting (pnpm format) over TypeScript and Markdown files
Files:
apps/web/src/lib/constants/plans.tsapps/web/src/server/mailer.tsapps/web/src/server/service/domain-service.tsAGENTS.mdapps/web/src/server/billing/payments.tsapps/web/src/server/redis.tsapps/web/src/server/email-templates/components/EmailFooter.tsxapps/web/src/server/api/routers/billing.tsapps/web/src/server/api/routers/invitiation.tsapps/web/src/server/email-templates/UsageWarningEmail.tsxapps/web/src/server/email-templates/index.tsapps/web/src/server/service/contact-book-service.tsapps/web/src/server/service/email-queue-service.tsapps/web/src/server/email-templates/UsageLimitReachedEmail.tsxapps/web/src/server/api/routers/limits.tsapps/web/src/server/service/email-service.tsapps/web/src/server/service/limit-service.tsapps/web/src/app/api/dev/email-preview/route.tsapps/web/src/server/service/team-service.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
In apps/web, use the import alias "
/" for src imports (e.g., import { x } from "/utils/x")
Files:
apps/web/src/lib/constants/plans.tsapps/web/src/server/mailer.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/billing/payments.tsapps/web/src/server/redis.tsapps/web/src/server/email-templates/components/EmailFooter.tsxapps/web/src/server/api/routers/billing.tsapps/web/src/server/api/routers/invitiation.tsapps/web/src/server/email-templates/UsageWarningEmail.tsxapps/web/src/server/email-templates/index.tsapps/web/src/server/service/contact-book-service.tsapps/web/src/server/service/email-queue-service.tsapps/web/src/server/email-templates/UsageLimitReachedEmail.tsxapps/web/src/server/api/routers/limits.tsapps/web/src/server/service/email-service.tsapps/web/src/server/service/limit-service.tsapps/web/src/app/api/dev/email-preview/route.tsapps/web/src/server/service/team-service.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase (e.g., AppSideBar.tsx)
Files:
apps/web/src/server/email-templates/components/EmailFooter.tsxapps/web/src/server/email-templates/UsageWarningEmail.tsxapps/web/src/server/email-templates/UsageLimitReachedEmail.tsx
🧬 Code graph analysis (11)
apps/web/src/server/service/domain-service.ts (1)
apps/web/src/server/db.ts (1)
db(20-20)
apps/web/src/server/billing/payments.ts (1)
apps/web/src/server/service/team-service.ts (1)
TeamService(17-482)
apps/web/src/server/api/routers/billing.ts (1)
apps/web/src/server/service/team-service.ts (1)
TeamService(17-482)
apps/web/src/server/email-templates/UsageWarningEmail.tsx (4)
apps/web/src/server/email-templates/components/EmailLayout.tsx (1)
EmailLayout(16-77)apps/web/src/server/email-templates/components/EmailHeader.tsx (1)
EmailHeader(9-45)apps/web/src/server/email-templates/components/EmailButton.tsx (1)
EmailButton(10-38)apps/web/src/server/email-templates/components/EmailFooter.tsx (1)
EmailFooter(9-43)
apps/web/src/server/service/contact-book-service.ts (1)
apps/web/src/server/db.ts (1)
db(20-20)
apps/web/src/server/service/email-queue-service.ts (2)
apps/web/src/server/service/limit-service.ts (1)
LimitService(13-174)apps/web/src/server/service/team-service.ts (1)
TeamService(17-482)
apps/web/src/server/email-templates/UsageLimitReachedEmail.tsx (5)
apps/web/src/server/email-templates/index.ts (2)
UsageLimitReachedEmail(8-8)renderUsageLimitReachedEmail(9-9)apps/web/src/server/email-templates/components/EmailLayout.tsx (1)
EmailLayout(16-77)apps/web/src/server/email-templates/components/EmailHeader.tsx (1)
EmailHeader(9-45)apps/web/src/server/email-templates/components/EmailButton.tsx (1)
EmailButton(10-38)apps/web/src/server/email-templates/components/EmailFooter.tsx (1)
EmailFooter(9-43)
apps/web/src/server/service/email-service.ts (1)
apps/web/src/types/index.ts (1)
EmailContent(1-17)
apps/web/src/server/service/limit-service.ts (5)
apps/web/src/env.js (2)
env(5-125)env(5-125)apps/web/src/server/service/team-service.ts (1)
TeamService(17-482)apps/web/src/lib/constants/plans.ts (1)
PLAN_LIMITS(12-36)apps/web/src/server/redis.ts (1)
withCache(19-50)apps/web/src/server/service/usage-service.ts (1)
getThisMonthUsage(12-63)
apps/web/src/app/api/dev/email-preview/route.ts (3)
apps/web/src/server/email-templates/UsageWarningEmail.tsx (1)
renderUsageWarningEmail(112-116)apps/web/src/server/email-templates/index.ts (2)
renderUsageWarningEmail(5-5)renderUsageLimitReachedEmail(9-9)apps/web/src/server/email-templates/UsageLimitReachedEmail.tsx (1)
renderUsageLimitReachedEmail(106-110)
apps/web/src/server/service/team-service.ts (7)
apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/redis.ts (1)
getRedis(6-13)apps/web/src/lib/constants/plans.ts (1)
PLAN_LIMITS(12-36)apps/web/src/env.js (2)
env(5-125)env(5-125)apps/web/src/server/mailer.ts (1)
sendMail(72-132)apps/web/src/server/email-templates/UsageWarningEmail.tsx (1)
renderUsageWarningEmail(112-116)apps/web/src/server/email-templates/UsageLimitReachedEmail.tsx (1)
renderUsageLimitReachedEmail(106-110)
🔇 Additional comments (26)
apps/web/src/server/service/email-service.ts (2)
565-581: Grouping by from only can mix teams in bulk sendsIf emailContents span multiple teams with the same “from”, the first team’s validated domain will be reused for others. If the caller invariant is “single team per call”, add a guard; otherwise, key by teamId+from.
- const emailsByDomain = new Map< - string, + const emailsByDomain = new Map< + string, { domain: Awaited<ReturnType<typeof validateDomainFromEmail>>; emails: typeof filteredEmailContents; } >(); ... - if (!emailsByDomain.has(from)) { - const domain = await validateDomainFromEmail(from, content.teamId); - emailsByDomain.set(from, { domain, emails: [] }); + const key = `${content.teamId}:${from}`; + if (!emailsByDomain.has(key)) { + const domain = await validateDomainFromEmail(from, content.teamId); + emailsByDomain.set(key, { domain, emails: [] }); } - emailsByDomain.get(from)?.emails.push(content); + emailsByDomain.get(key)?.emails.push(content);If “single team per call” is guaranteed, consider asserting it early and logging if violated.
1-9: Formatting-only changes look goodapps/web/src/server/service/contact-book-service.ts (2)
31-40: LGTM: explicit await and return improve clarityBehavior is unchanged; naming is clear.
84-87: LGTM: consistent pattern on deleteConsistent with create path; fine to keep.
apps/web/prisma/schema.prisma (2)
110-113: Team flags and limit field additions look goodDefaults are sensible and align with limit enforcement.
110-113: Verify migration impact and plan alignment
- Ensure the migration backfills defaults without locking tables for long on large Team datasets.
- Confirm dropped enum SendingDisabledReason has no remaining references.
- Validate dailyEmailLimit matches plan defaults in apps/web/src/lib/constants/plans.ts.
apps/web/src/server/email-templates/components/EmailFooter.tsx (1)
9-12: Good default: mailto support linkNice UX tweak; no issues.
apps/web/prisma/migrations/20250907195449_add_team_verification_and_limits/migration.sql (2)
2-4: Sanity-check defaults vs. plan logic.
dailyEmailLimit DEFAULT 10000may exceed FREE plan daily limits (100). Confirm enforcement chooses the stricter of Plan vs Team override or adjust the default here.
6-7: Safe to drop enum: no usages ofSendingDisabledReasonremain in schema or migrations; dropping the type is safe.apps/web/src/server/billing/payments.ts (2)
18-21: Double-checkbillingEmailsource.
customer.emailwill usually be null here since the customer was created without an email. EnsureTeam.billingEmailaccepts null or populate from team/user data instead.
184-190: LGTM on centralizing team updates with cache refresh.Keeps plan/isActive updates consistent and cache-coherent.
apps/web/src/server/email-templates/index.ts (2)
3-10: Exports look correct and consistent.Templates and render helpers are exposed as expected.
15-15: Re-export of EmailButton is fine.Keeps components discoverable from a single barrel.
apps/web/src/app/api/dev/email-preview/route.ts (1)
5-6: LGTM: new template importsImports are correct and follow "~/" alias.
apps/web/src/server/service/team-service.ts (5)
83-97: Create + warm cache looks goodWarming the cache on team creation reduces first-hit latency.
99-110: Update + refresh cache is correctRefreshing after write-through keeps readers consistent.
242-256: Invalidate cache after role change is appropriateSafer than refresh here given potential permission fan-out.
300-309: Invalidate cache after member delete is appropriatePrevents stale plan/limits visibility.
484-502: Helper looks goodCentralizing limit-reached email composition keeps subjects/bodies consistent.
apps/web/src/server/service/limit-service.ts (7)
19-22: Cloud gating LGTMShort-circuiting for self-hosted keeps behavior aligned with product split.
24-26: Count via DB + plan-based limit is correctUsing TeamService cache for plan is appropriate.
48-50: Contact book cloud gating LGTM
75-78: Team member cloud gating LGTM
98-107: Email gating only in cloud LGTM
109-118: Blocked teams: immediate 0 limit is correctClear, explicit reason code.
169-172: Return shape consistencyGood to include available on success responses. Ensure callers handle -1 (unlimited) and compute UI accordingly.
| .input( | ||
| z.object({ | ||
| type: z.nativeEnum(LimitReason), | ||
| }), | ||
| }) | ||
| ) |
There was a problem hiding this comment.
Input schema admits email reasons that the switch no longer handles.
Clients sending any EMAIL_* reason will now hit the default error. Narrow the schema to supported types or handle the new reasons explicitly.
- .input(
- z.object({
- type: z.nativeEnum(LimitReason),
- })
- )
+ .input(
+ z.object({
+ type: z.enum(["CONTACT_BOOK", "DOMAIN", "TEAM_MEMBER"]),
+ })
+ )📝 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.
| .input( | |
| z.object({ | |
| type: z.nativeEnum(LimitReason), | |
| }), | |
| }) | |
| ) | |
| .input( | |
| z.object({ | |
| type: z.enum(["CONTACT_BOOK", "DOMAIN", "TEAM_MEMBER"]), | |
| }) | |
| ) |
🤖 Prompt for AI Agents
In apps/web/src/server/api/routers/limits.ts around lines 8–12, the input schema
uses z.nativeEnum(LimitReason) which currently admits EMAIL_* reasons that the
handler's switch no longer supports; either narrow the schema to only the
supported LimitReason values (replace the nativeEnum with an explicit enum/list
of allowed values or validate/transform to reject EMAIL_*), or update the switch
to explicitly handle the EMAIL_* reasons; implement one of these fixes and
return a clear validation error for unsupported reasons.
| const redis = getRedis(); | ||
|
|
||
| if (!disable) { | ||
| const cached = await redis.get(key); | ||
| if (cached) { | ||
| try { | ||
| return JSON.parse(cached) as T; | ||
| } catch { | ||
| // fallthrough to refresh cache | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fail-open on cache read errors.
If Redis is down, redis.get will throw and bubble up. Swallow read errors so callers aren’t taken down by cache.
- const redis = getRedis();
-
- if (!disable) {
- const cached = await redis.get(key);
+ // Defer connect and fail-open on read errors
+ let cached: string | null = null;
+ if (!disable) {
+ const redis = getRedis();
+ try {
+ cached = await redis.get(key);
+ } catch {
+ // ignore cache read errors
+ }
if (cached) {
try {
return JSON.parse(cached) as T;
} catch {
// fallthrough to refresh cache
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const redis = getRedis(); | |
| if (!disable) { | |
| const cached = await redis.get(key); | |
| if (cached) { | |
| try { | |
| return JSON.parse(cached) as T; | |
| } catch { | |
| // fallthrough to refresh cache | |
| } | |
| } | |
| } | |
| // Defer connect and fail-open on read errors | |
| let cached: string | null = null; | |
| if (!disable) { | |
| const redis = getRedis(); | |
| try { | |
| cached = await redis.get(key); | |
| } catch { | |
| // ignore cache read errors | |
| } | |
| if (cached) { | |
| try { | |
| return JSON.parse(cached) as T; | |
| } catch { | |
| // fallthrough to refresh cache | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/server/redis.ts around lines 26 to 37, the current code calls
redis.get(key) directly so if Redis is down the thrown error bubbles up; wrap
the read in a try/catch to fail-open: attempt to await redis.get(key) inside a
try block, handle any thrown error by logging a warning/debug message and
setting cached to undefined (so execution falls through to refresh the cache),
then proceed with the existing JSON.parse logic inside its own try/catch; ensure
only read errors are swallowed and no unhandled exceptions escape.
| if (limitCheck.limit !== -1 && limitCheck.available) { | ||
| if (limitCheck.available / limitCheck.limit < 0.2) { | ||
| await TeamService.sendWarningEmail( | ||
| email.teamId, | ||
| limitCheck.available, | ||
| limitCheck.limit, | ||
| limitCheck.reason | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Bug: warning email uses “available” as “used” and reason is likely undefined
- You’re passing available where TeamService expects used. That flips the metric.
- LimitService returns reason only when a limit is hit; here it’s undefined, so sendWarningEmail early-returns and no warning is sent.
Fix both by computing used and providing a concrete daily reason.
- if (limitCheck.limit !== -1 && limitCheck.available) {
- if (limitCheck.available / limitCheck.limit < 0.2) {
- await TeamService.sendWarningEmail(
- email.teamId,
- limitCheck.available,
- limitCheck.limit,
- limitCheck.reason
- );
- }
- }
+ if (limitCheck.limit !== -1 && typeof limitCheck.available === "number") {
+ const used = limitCheck.limit - limitCheck.available;
+ if (limitCheck.available / limitCheck.limit < 0.2) {
+ await TeamService.sendWarningEmail(
+ email.teamId,
+ used,
+ limitCheck.limit,
+ // Fallback to daily reason so TeamService actually sends the warning
+ limitCheck.reason ?? LimitReason.EMAIL_DAILY_LIMIT_REACHED
+ );
+ }
+ }Add the missing import (use the same source you use elsewhere for LimitReason):
🏁 Script executed:
#!/bin/bash
# Find where LimitReason is exported so you can import it here.
rg -nP 'export\s+(enum|const|type)\s+LimitReason\b|^export\s*{\s*LimitReason\s*}' -SLength of output: 63
Correct sendWarningEmail invocation and import LimitReason
- Compute
used = limitCheck.limit - limitCheck.availableand passusedas the second argument (instead ofavailable) - Provide a fallback reason:
limitCheck.reason ?? LimitReason.EMAIL_DAILY_LIMIT_REACHED - At the top of apps/web/src/server/service/email-queue-service.ts, add:
import { LimitReason } from '../../lib/constants/plans';
🤖 Prompt for AI Agents
In apps/web/src/server/service/email-queue-service.ts around lines 433 to 442,
the TeamService.sendWarningEmail call passes available and may omit a reason;
change it to compute used = limitCheck.limit - limitCheck.available and pass
used as the second argument, and pass limitCheck.reason ??
LimitReason.EMAIL_DAILY_LIMIT_REACHED as the reason; also add an import at the
top of the file: import { LimitReason } from '../../lib/constants/plans'; to
ensure the fallback reason is available.
| // Enforce daily sending limit (team-specific) | ||
| const usage = await withCache( | ||
| `usage:this-month:${teamId}`, | ||
| () => getThisMonthUsage(teamId), | ||
| { ttlSeconds: 60 } | ||
| ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
60s usage cache can allow oversend bursts
With a 60s TTL, high-throughput sends can exceed the daily cap before the next refresh. Consider lowering TTL (e.g., 5–10s) or disabling cache for daily checks.
Apply:
- { ttlSeconds: 60 }
+ { ttlSeconds: 10 }If needed, split keys: cache today’s usage at a lower TTL and month at a higher TTL, or enforce with Redis INCR counters for atomicity.
📝 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.
| // Enforce daily sending limit (team-specific) | |
| const usage = await withCache( | |
| `usage:this-month:${teamId}`, | |
| () => getThisMonthUsage(teamId), | |
| { ttlSeconds: 60 } | |
| ); | |
| // Enforce daily sending limit (team-specific) | |
| const usage = await withCache( | |
| `usage:this-month:${teamId}`, | |
| () => getThisMonthUsage(teamId), | |
| { ttlSeconds: 10 } | |
| ); |
🤖 Prompt for AI Agents
In apps/web/src/server/service/limit-service.ts around lines 120 to 126, the
current 60s TTL for caching this-month usage can permit brief oversend bursts;
change the caching strategy to avoid that by either lowering the TTL to 5–10
seconds or disabling the cache for daily/during-high-throughput checks, or split
keys so "usage:today:{teamId}" uses a low TTL (5–10s) while
"usage:this-month:{teamId}" can keep a longer TTL, and/or replace the cached
read with an atomic Redis counter (INCR/INCRBY + EXPIRE) for enforcing the daily
cap to ensure accurate, atomic increments under concurrency.
| const dailyUsage = usage.day.reduce((acc, curr) => acc + curr.sent, 0); | ||
| const dailyLimit = | ||
| team.plan !== "FREE" | ||
| ? team.dailyEmailLimit | ||
| : PLAN_LIMITS[team.plan].emailsPerDay; | ||
|
|
There was a problem hiding this comment.
Potential undefined dailyLimit for paid plans → unlimited send
If team.dailyEmailLimit is null/undefined, comparisons become falsey and return NaN for available. Default to plan cap (-1 for unlimited) or a sane configured value.
Apply:
- const dailyLimit =
- team.plan !== "FREE"
- ? team.dailyEmailLimit
- : PLAN_LIMITS[team.plan].emailsPerDay;
+ const planDaily = PLAN_LIMITS[team.plan].emailsPerDay; // -1 for unlimited on paid
+ const dailyLimit =
+ team.plan !== "FREE"
+ ? (team.dailyEmailLimit ?? planDaily)
+ : planDaily;Also consider validating dailyEmailLimit >= -1 at write-time.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const dailyUsage = usage.day.reduce((acc, curr) => acc + curr.sent, 0); | |
| const dailyLimit = | |
| team.plan !== "FREE" | |
| ? team.dailyEmailLimit | |
| : PLAN_LIMITS[team.plan].emailsPerDay; | |
| const dailyUsage = usage.day.reduce((acc, curr) => acc + curr.sent, 0); | |
| const planDaily = PLAN_LIMITS[team.plan].emailsPerDay; // -1 for unlimited on paid | |
| const dailyLimit = | |
| team.plan !== "FREE" | |
| - ? team.dailyEmailLimit | |
| ? (team.dailyEmailLimit ?? planDaily) | |
| : planDaily; |
🤖 Prompt for AI Agents
In apps/web/src/server/service/limit-service.ts around lines 127 to 132, the
code assumes team.dailyEmailLimit is defined for paid plans which can yield
undefined/NaN when computing available sends; change the logic to default
dailyLimit to a safe value when team.dailyEmailLimit is null/undefined (use
team.dailyEmailLimit ?? PLAN_LIMITS[team.plan].emailsPerDay, and treat special
value -1 as unlimited), ensure comparisons treat -1 as unlimited (skip numeric
math when -1), and add a note to validate at write-time that dailyEmailLimit is
an integer >= -1 to prevent invalid values from being saved.
| static async getTeamCached(teamId: number): Promise<Team> { | ||
| const redis = getRedis(); | ||
| const raw = await redis.get(TeamService.cacheKey(teamId)); | ||
| if (raw) { | ||
| return JSON.parse(raw) as Team; | ||
| } | ||
| const fresh = await TeamService.refreshTeamCache(teamId); | ||
| if (!fresh) { | ||
| throw new TRPCError({ code: "NOT_FOUND", message: "Team not found" }); | ||
| } | ||
| return fresh; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden cache read against corruption
A bad/partial cache value will throw on JSON.parse and 500 the request. Fall back to DB on parse error.
Apply:
- const raw = await redis.get(TeamService.cacheKey(teamId));
- if (raw) {
- return JSON.parse(raw) as Team;
- }
+ const raw = await redis.get(TeamService.cacheKey(teamId));
+ if (raw) {
+ try {
+ return JSON.parse(raw) as Team;
+ } catch {
+ // fall through to refresh
+ }
+ }📝 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.
| static async getTeamCached(teamId: number): Promise<Team> { | |
| const redis = getRedis(); | |
| const raw = await redis.get(TeamService.cacheKey(teamId)); | |
| if (raw) { | |
| return JSON.parse(raw) as Team; | |
| } | |
| const fresh = await TeamService.refreshTeamCache(teamId); | |
| if (!fresh) { | |
| throw new TRPCError({ code: "NOT_FOUND", message: "Team not found" }); | |
| } | |
| return fresh; | |
| } | |
| static async getTeamCached(teamId: number): Promise<Team> { | |
| const redis = getRedis(); | |
| const raw = await redis.get(TeamService.cacheKey(teamId)); | |
| if (raw) { | |
| try { | |
| return JSON.parse(raw) as Team; | |
| } catch { | |
| // fall through to refresh | |
| } | |
| } | |
| const fresh = await TeamService.refreshTeamCache(teamId); | |
| if (!fresh) { | |
| throw new TRPCError({ code: "NOT_FOUND", message: "Team not found" }); | |
| } | |
| return fresh; | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/server/service/team-service.ts around lines 41 to 52, the
current JSON.parse(raw) will throw on malformed/partial cache values and cause a
500; wrap the parse in a try/catch, and on parse error log a warning (use the
existing logger or console.warn), delete the corrupted cache key via
redis.del(TeamService.cacheKey(teamId)), then fall back to fetching fresh data
by calling await TeamService.refreshTeamCache(teamId) and return that (or throw
the NOT_FOUND TRPCError if refresh returns null); ensure no raw parse exception
bubbles up.
| const cachedTeam = await TeamService.getTeamCached(teamId); | ||
| const memberLimit = PLAN_LIMITS[cachedTeam.plan].teamMembers; | ||
| const currentMembers = await db.teamUser.count({ where: { teamId } }); | ||
| const isExceeded = memberLimit !== -1 && currentMembers >= memberLimit; | ||
| if (isExceeded) { | ||
| throw new UnsendApiError({ | ||
| code: "FORBIDDEN", | ||
| message: reason ?? "Team invite limit reached", | ||
| message: "Team invite limit reached", | ||
| }); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Invite limit should consider pending invites and race conditions
You only count current members; multiple concurrent invites can exceed the limit and later fail on accept, creating a poor UX. Include pending invites in the check (and ideally wrap in a transaction).
Proposed change:
- const currentMembers = await db.teamUser.count({ where: { teamId } });
- const isExceeded = memberLimit !== -1 && currentMembers >= memberLimit;
+ const [currentMembers, pendingInvites] = await Promise.all([
+ db.teamUser.count({ where: { teamId } }),
+ db.teamInvite.count({ where: { teamId } }),
+ ]);
+ const projected = currentMembers + pendingInvites;
+ const isExceeded = memberLimit !== -1 && projected >= memberLimit;If Prisma supports it in your DB, consider enforcing via a CHECK or handling at acceptance time with a unique constraint-like guard.
🏁 Script executed:
#!/bin/bash
set -e
# Locate the Prisma schema file
schema_file=$(find . -type f -name '*.prisma' | head -n1)
echo "Schema file: $schema_file"
if [ -n "$schema_file" ]; then
grep -R "model TeamInvite" -n "$schema_file" || echo "No TeamInvite model found in schema"
else
echo "No .prisma schema file found"
fi
# Search codebase for teamInvite usage
rg -n "teamInvite" -C2 .Length of output: 5705
🏁 Script executed:
#!/bin/bash
set -e
echo "Schema TeamInvite model:"
rg -n 'model TeamInvite' -C5 apps/web/prisma/schema.prisma || true
echo "--- getTeamInvites function ---"
rg -n 'static async getTeamInvites' -C5 apps/web/src/server/service/team-service.ts || true
echo "--- Invite limit snippet (lines 160-180) ---"
sed -n '160,180p' apps/web/src/server/service/team-service.ts || trueLength of output: 1225
Include pending invites in the team invite limit check and guard against races
apps/web/src/server/service/team-service.ts:164–173
- Add a count of pending invites before enforcing
memberLimit, e.g.:const currentMembers = await db.teamUser.count({ where: { teamId } }); // ➜ include outstanding invites in the projection - const isExceeded = memberLimit !== -1 && currentMembers >= memberLimit;
- const [pendingInvites] = await Promise.all([
-
db.teamInvite.count({ where: { teamId } }), - ]);
- const isExceeded = memberLimit !== -1
-
&& currentMembers + pendingInvites >= memberLimit;
- Wrap the count → create sequence in a
db.$transaction(or enforce via a DB-level constraint) to prevent concurrent invites from bypassing the limit.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const cachedTeam = await TeamService.getTeamCached(teamId); | |
| const memberLimit = PLAN_LIMITS[cachedTeam.plan].teamMembers; | |
| const currentMembers = await db.teamUser.count({ where: { teamId } }); | |
| const isExceeded = memberLimit !== -1 && currentMembers >= memberLimit; | |
| if (isExceeded) { | |
| throw new UnsendApiError({ | |
| code: "FORBIDDEN", | |
| message: reason ?? "Team invite limit reached", | |
| message: "Team invite limit reached", | |
| }); | |
| } | |
| const cachedTeam = await TeamService.getTeamCached(teamId); | |
| const memberLimit = PLAN_LIMITS[cachedTeam.plan].teamMembers; | |
| const currentMembers = await db.teamUser.count({ where: { teamId } }); | |
| // ➜ include outstanding invites in the projection | |
| const [pendingInvites] = await Promise.all([ | |
| db.teamInvite.count({ where: { teamId } }), | |
| ]); | |
| const isExceeded = memberLimit !== -1 | |
| && currentMembers + pendingInvites >= memberLimit; | |
| if (isExceeded) { | |
| throw new UnsendApiError({ | |
| code: "FORBIDDEN", | |
| message: "Team invite limit reached", | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/server/service/team-service.ts around lines 164 to 173, the
current limit check only counts existing team members and can be bypassed by
pending invites or concurrent requests; update the logic to include pending
invites in the tally (e.g., count team_user rows plus pending team_invite rows
for the team, treating PLAN_LIMITS[plan] === -1 as unlimited) and then enforce
the limit against (currentMembers + pendingInvites). To prevent race conditions,
perform the counting and the subsequent invite creation inside a single
db.$transaction (or use a DB-level locking/select-for-update) so the check and
insert are atomic; if the combined count would exceed the limit, throw the
UnsendApiError and never create the invite.
| /** | ||
| * Notify all team users that email limit has been reached, at most once per 6 hours. | ||
| */ | ||
| static async maybeNotifyEmailLimitReached( | ||
| teamId: number, | ||
| limit: number, | ||
| reason: LimitReason | undefined | ||
| ) { | ||
| if (!reason) return; | ||
| // Only notify on actual email limit reasons | ||
| if ( | ||
| ![ | ||
| LimitReason.EMAIL_DAILY_LIMIT_REACHED, | ||
| LimitReason.EMAIL_FREE_PLAN_MONTHLY_LIMIT_REACHED, | ||
| ].includes(reason) | ||
| ) | ||
| return; | ||
|
|
||
| const redis = getRedis(); | ||
| const cacheKey = `limit:notify:${teamId}:${reason}`; | ||
| const alreadySent = await redis.get(cacheKey); | ||
| if (alreadySent) { | ||
| return; // within cooldown window | ||
| } | ||
|
|
||
| const team = await TeamService.getTeamCached(teamId); | ||
| const isPaidPlan = team.plan !== "FREE"; | ||
|
|
||
| const html = await getLimitReachedEmail(teamId, limit, reason); | ||
|
|
||
| const subject = | ||
| reason === LimitReason.EMAIL_FREE_PLAN_MONTHLY_LIMIT_REACHED | ||
| ? "You've reached your monthly email limit" | ||
| : "You've reached your daily email limit"; | ||
|
|
||
| const text = `Hi ${team.name} team,\n\nYou've reached your ${ | ||
| reason === LimitReason.EMAIL_FREE_PLAN_MONTHLY_LIMIT_REACHED | ||
| ? "monthly" | ||
| : "daily" | ||
| } limit of ${limit.toLocaleString()} emails.\n\nSending is temporarily paused until your limit resets or ${ | ||
| isPaidPlan ? "your team is verified" : "your plan is upgraded" | ||
| }.\n\nManage plan: ${env.NEXTAUTH_URL}/settings`; | ||
|
|
||
| const teamUsers = await TeamService.getTeamUsers(teamId); | ||
| const recipients = teamUsers | ||
| .map((tu) => tu.user?.email) | ||
| .filter((e): e is string => Boolean(e)); | ||
|
|
||
| // Send individually to all team users | ||
| await Promise.all( | ||
| recipients.map((to) => sendMail(to, subject, text, html)) | ||
| ); | ||
|
|
||
| // Set cooldown for 6 hours | ||
| await redis.setex(cacheKey, 6 * 60 * 60, "1"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Deduplicate notifications atomically; set cooldown before sending
Two workers can pass the get() check and double-send. Use SET NX EX to gate, then send. Also guard empty recipient lists and swallow per-recipient errors to still set cooldown.
Apply:
- const alreadySent = await redis.get(cacheKey);
- if (alreadySent) {
- return; // within cooldown window
- }
+ const acquired = await redis.set(cacheKey, "1", "NX", "EX", 6 * 60 * 60);
+ if (!acquired) return; // within cooldown window
@@
- // Send individually to all team users
- await Promise.all(
- recipients.map((to) => sendMail(to, subject, text, html))
- );
-
- // Set cooldown for 6 hours
- await redis.setex(cacheKey, 6 * 60 * 60, "1");
+ if (recipients.length === 0) return;
+ await Promise.allSettled(
+ recipients.map((to) => sendMail(to!, subject, text, html))
+ );🤖 Prompt for AI Agents
In apps/web/src/server/service/team-service.ts around lines 360 to 415, avoid
double-sends by atomically setting the cooldown key before sending: replace the
current get() + setex() flow with a single SET NX EX (set with NX and EX) that
attempts to create the cache key for 6 hours and returns early if it already
exists; only proceed to prepare/send emails when the SET NX EX succeeds, and
still ensure the cooldown is set even if individual sends fail. Also guard
against empty recipient lists (return early if none) and wrap per-recipient
sendMail calls so failures are caught and logged/swallowed without aborting the
whole run, but do not remove the cooldown on send errors.
| static async sendWarningEmail( | ||
| teamId: number, | ||
| used: number, | ||
| limit: number, | ||
| reason: LimitReason | undefined | ||
| ) { | ||
| if (!reason) return; | ||
| // Only warn for email usage-related reasons (daily or monthly free plan) | ||
| if ( | ||
| ![ | ||
| LimitReason.EMAIL_DAILY_LIMIT_REACHED, | ||
| LimitReason.EMAIL_FREE_PLAN_MONTHLY_LIMIT_REACHED, | ||
| ].includes(reason) | ||
| ) | ||
| return; | ||
|
|
||
| const redis = getRedis(); | ||
| const cacheKey = `limit:warning:${teamId}:${reason}`; | ||
| const alreadySent = await redis.get(cacheKey); | ||
| if (alreadySent) { | ||
| return; // within cooldown window | ||
| } | ||
|
|
||
| const team = await TeamService.getTeamCached(teamId); | ||
| const isPaidPlan = team.plan !== "FREE"; | ||
|
|
||
| const period = | ||
| reason === LimitReason.EMAIL_FREE_PLAN_MONTHLY_LIMIT_REACHED | ||
| ? "monthly" | ||
| : "daily"; | ||
|
|
||
| const html = await renderUsageWarningEmail({ | ||
| teamName: team.name, | ||
| used, | ||
| limit, | ||
| isPaidPlan, | ||
| period, | ||
| manageUrl: `${env.NEXTAUTH_URL}/settings`, | ||
| }); | ||
|
|
||
| const subject = | ||
| period === "monthly" | ||
| ? "You're nearing your monthly email limit" | ||
| : "You're nearing your daily email limit"; | ||
|
|
||
| const text = `Hi ${team.name} team,\n\nYou've used ${used.toLocaleString()} of your ${period} limit of ${limit.toLocaleString()} emails.\n\nConsider ${ | ||
| isPaidPlan | ||
| ? "verifying your team by replying to this email" | ||
| : "upgrading your plan" | ||
| }.\n\nManage plan: ${env.NEXTAUTH_URL}/settings`; | ||
|
|
||
| const teamUsers = await TeamService.getTeamUsers(teamId); | ||
| const recipients = teamUsers | ||
| .map((tu) => tu.user?.email) | ||
| .filter((e): e is string => Boolean(e)); | ||
|
|
||
| await Promise.all(recipients.map((to) => sendMail(to, subject, text, html))); | ||
|
|
||
| // Set cooldown for 6 hours | ||
| await redis.setex(cacheKey, 6 * 60 * 60, "1"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply same atomic cooldown and error-handling to warning emails
Mirror the SET NX EX pattern; avoid duplicate warnings under load.
Apply:
- const alreadySent = await redis.get(cacheKey);
- if (alreadySent) {
- return; // within cooldown window
- }
+ const acquired = await redis.set(cacheKey, "1", "NX", "EX", 6 * 60 * 60);
+ if (!acquired) return;
@@
- await Promise.all(recipients.map((to) => sendMail(to, subject, text, html)));
-
- // Set cooldown for 6 hours
- await redis.setex(cacheKey, 6 * 60 * 60, "1");
+ if (recipients.length === 0) return;
+ await Promise.allSettled(
+ recipients.map((to) => sendMail(to!, subject, text, html))
+ );📝 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.
| static async sendWarningEmail( | |
| teamId: number, | |
| used: number, | |
| limit: number, | |
| reason: LimitReason | undefined | |
| ) { | |
| if (!reason) return; | |
| // Only warn for email usage-related reasons (daily or monthly free plan) | |
| if ( | |
| ![ | |
| LimitReason.EMAIL_DAILY_LIMIT_REACHED, | |
| LimitReason.EMAIL_FREE_PLAN_MONTHLY_LIMIT_REACHED, | |
| ].includes(reason) | |
| ) | |
| return; | |
| const redis = getRedis(); | |
| const cacheKey = `limit:warning:${teamId}:${reason}`; | |
| const alreadySent = await redis.get(cacheKey); | |
| if (alreadySent) { | |
| return; // within cooldown window | |
| } | |
| const team = await TeamService.getTeamCached(teamId); | |
| const isPaidPlan = team.plan !== "FREE"; | |
| const period = | |
| reason === LimitReason.EMAIL_FREE_PLAN_MONTHLY_LIMIT_REACHED | |
| ? "monthly" | |
| : "daily"; | |
| const html = await renderUsageWarningEmail({ | |
| teamName: team.name, | |
| used, | |
| limit, | |
| isPaidPlan, | |
| period, | |
| manageUrl: `${env.NEXTAUTH_URL}/settings`, | |
| }); | |
| const subject = | |
| period === "monthly" | |
| ? "You're nearing your monthly email limit" | |
| : "You're nearing your daily email limit"; | |
| const text = `Hi ${team.name} team,\n\nYou've used ${used.toLocaleString()} of your ${period} limit of ${limit.toLocaleString()} emails.\n\nConsider ${ | |
| isPaidPlan | |
| ? "verifying your team by replying to this email" | |
| : "upgrading your plan" | |
| }.\n\nManage plan: ${env.NEXTAUTH_URL}/settings`; | |
| const teamUsers = await TeamService.getTeamUsers(teamId); | |
| const recipients = teamUsers | |
| .map((tu) => tu.user?.email) | |
| .filter((e): e is string => Boolean(e)); | |
| await Promise.all(recipients.map((to) => sendMail(to, subject, text, html))); | |
| // Set cooldown for 6 hours | |
| await redis.setex(cacheKey, 6 * 60 * 60, "1"); | |
| } | |
| static async sendWarningEmail( | |
| teamId: number, | |
| used: number, | |
| limit: number, | |
| reason: LimitReason | undefined | |
| ) { | |
| if (!reason) return; | |
| // Only warn for email usage-related reasons (daily or monthly free plan) | |
| if ( | |
| ![ | |
| LimitReason.EMAIL_DAILY_LIMIT_REACHED, | |
| LimitReason.EMAIL_FREE_PLAN_MONTHLY_LIMIT_REACHED, | |
| ].includes(reason) | |
| ) | |
| return; | |
| const redis = getRedis(); | |
| const cacheKey = `limit:warning:${teamId}:${reason}`; | |
| // Atomically set cooldown; if already set, bail out immediately | |
| const acquired = await redis.set(cacheKey, "1", "NX", "EX", 6 * 60 * 60); | |
| if (!acquired) return; | |
| const team = await TeamService.getTeamCached(teamId); | |
| const isPaidPlan = team.plan !== "FREE"; | |
| const period = | |
| reason === LimitReason.EMAIL_FREE_PLAN_MONTHLY_LIMIT_REACHED | |
| ? "monthly" | |
| : "daily"; | |
| const html = await renderUsageWarningEmail({ | |
| teamName: team.name, | |
| used, | |
| limit, | |
| isPaidPlan, | |
| period, | |
| manageUrl: `${env.NEXTAUTH_URL}/settings`, | |
| }); | |
| const subject = | |
| period === "monthly" | |
| ? "You're nearing your monthly email limit" | |
| : "You're nearing your daily email limit"; | |
| const text = `Hi ${team.name} team,\n\nYou've used ${used.toLocaleString()} of your ${period} limit of ${limit.toLocaleString()} emails.\n\nConsider ${ | |
| isPaidPlan | |
| ? "verifying your team by replying to this email" | |
| : "upgrading your plan" | |
| }.\n\nManage plan: ${env.NEXTAUTH_URL}/settings`; | |
| const teamUsers = await TeamService.getTeamUsers(teamId); | |
| const recipients = teamUsers | |
| .map((tu) => tu.user?.email) | |
| .filter((e): e is string => Boolean(e)); | |
| // Nothing to do if there are no recipients | |
| if (recipients.length === 0) return; | |
| // Send all mails without failing on the first error | |
| await Promise.allSettled( | |
| recipients.map((to) => sendMail(to!, subject, text, html)) | |
| ); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/server/mailer.ts (1)
113-134: Fix cloud send gating: using only UNSEND_API_KEY breaks when only USESEND_API_KEY is setCondition checks only
UNSEND_API_KEYwhilegetClient()supports either key. Also, the thrown error message blames missing API keys even whenFROM_EMAILis the culprit, and the log mentions a “fallback to resend” that doesn’t exist.Apply:
- } else if (env.UNSEND_API_KEY && env.FROM_EMAIL) { + } else if ((env.USESEND_API_KEY || env.UNSEND_API_KEY) && env.FROM_EMAIL) { const resp = await getClient().emails.send({ to: email, from: env.FROM_EMAIL, subject, text, html, replyTo, }); if (resp.data) { logger.info("Email sent using usesend"); return; } else { logger.error( { code: resp.error?.code, message: resp.error?.message }, - "Error sending email using usesend, so fallback to resend" + "Error sending email using usesend" ); } } else { - throw new Error("USESEND_API_KEY/UNSEND_API_KEY not found"); + throw new Error("Missing USESEND_API_KEY or UNSEND_API_KEY, or FROM_EMAIL"); }
♻️ Duplicate comments (6)
apps/web/src/server/service/team-service.ts (4)
41-52: Harden cache read: handle corrupted JSON and refresh
JSON.parsecan throw and 500 the request. Fallback to DB and refresh cache; clear the bad key.static async getTeamCached(teamId: number): Promise<Team> { const redis = getRedis(); - const raw = await redis.get(TeamService.cacheKey(teamId)); - if (raw) { - return JSON.parse(raw) as Team; - } + const raw = await redis.get(TeamService.cacheKey(teamId)); + if (raw) { + try { + return JSON.parse(raw) as Team; + } catch { + logger.warn({ teamId }, "Corrupt team cache; refreshing"); + await redis.del(TeamService.cacheKey(teamId)); + } + } const fresh = await TeamService.refreshTeamCache(teamId); if (!fresh) { throw new TRPCError({ code: "NOT_FOUND", message: "Team not found" }); } return fresh; }
164-173: Include pending invites and guard against races in member limit checkOnly counting members allows exceeding the cap via outstanding invites and concurrent requests.
- const currentMembers = await db.teamUser.count({ where: { teamId } }); - const isExceeded = memberLimit !== -1 && currentMembers >= memberLimit; + const [currentMembers, pendingInvites] = await Promise.all([ + db.teamUser.count({ where: { teamId } }), + db.teamInvite.count({ where: { teamId } }), + ]); + const projected = currentMembers + pendingInvites; + const isExceeded = memberLimit !== -1 && projected >= memberLimit; if (isExceeded) { throw new UnsendApiError({ code: "FORBIDDEN", message: "Team invite limit reached", }); }Follow-up: wrap the count → create sequence in a single
db.$transactionto make the check atomic.Also applies to: 191-199
363-417: Deduplicate “limit reached” notifications atomically; don’t fail on partial sendsUse SET NX EX to gate before sending and Promise.allSettled to avoid aborting on per-recipient errors.
- const alreadySent = await redis.get(cacheKey); - if (alreadySent) { - return; // within cooldown window - } + const acquired = await redis.set(cacheKey, "1", "NX", "EX", 6 * 60 * 60); + if (acquired !== "OK") return; // within cooldown window @@ - // Send individually to all team users - await Promise.all( - recipients.map((to) => - sendMail(to, subject, text, html, "hey@usesend.com") - ) - ); - - // Set cooldown for 6 hours - await redis.setex(cacheKey, 6 * 60 * 60, "1"); + if (recipients.length === 0) return; + await Promise.allSettled( + recipients.map((to) => + sendMail(to!, subject, text, html, "hey@usesend.com") + ) + );
423-501: Apply the same atomic cooldown + resilient send for warning emailsMirror the NX/EX gate and use
Promise.allSettled.- const alreadySent = await redis.get(cacheKey); - if (alreadySent) { - return; // within cooldown window - } + const acquired = await redis.set(cacheKey, "1", "NX", "EX", 6 * 60 * 60); + if (acquired !== "OK") return; @@ - await Promise.all( - recipients.map((to) => - sendMail(to, subject, text, html, "hey@usesend.com") - ) - ); - - // Set cooldown for 6 hours - await redis.setex(cacheKey, 6 * 60 * 60, "1"); + if (recipients.length === 0) return; + await Promise.allSettled( + recipients.map((to) => + sendMail(to!, subject, text, html, "hey@usesend.com") + ) + );apps/web/src/server/service/limit-service.ts (2)
125-131: Lower usage cache TTL to reduce oversend windows60s cache can allow brief bursts past the cap; 10s is safer for daily checks.
- { ttlSeconds: 60 } + { ttlSeconds: 10 }Consider moving daily enforcement to atomic Redis counters for strict guarantees.
133-137: Default dailyLimit for paid plans when team.dailyEmailLimit is null/undefinedPrevents NaN math and false negatives in
isLimitExceeded.- const dailyLimit = - team.plan !== "FREE" - ? team.dailyEmailLimit - : PLAN_LIMITS[team.plan].emailsPerDay; + const planDaily = PLAN_LIMITS[team.plan].emailsPerDay; + const dailyLimit = + team.plan !== "FREE" ? (team.dailyEmailLimit ?? planDaily) : planDaily;
🧹 Nitpick comments (7)
apps/web/src/server/service/email-queue-service.ts (3)
312-313: Reduce log verbosity/redact job payload
job.datacan hold recipient/unsub info. Prefer debug level and log only identifiers.- logger.info({ jobData: job.data }, `[EmailQueueService]: Job data`); + logger.debug( + { emailId: job.data.emailId, teamId: job.data.teamId, isBulk: job.data.isBulk }, + `[EmailQueueService]: Job data` + );
372-395: Pre-send limit check is good; consider atomic “in-flight” gating to reduce race oversendMultiple workers can pass the check concurrently (usage cache lag). Consider a short-lived Redis INCR/EX key (e.g., usage:today:inflight:{teamId}) to tentatively reserve sends, decrement on failure, and block when reservation would exceed available.
426-429: Remove confusing boolean in log or make it structuredTemplate logs
true/falsefrom an expression. Either drop or log fields explicitly.- logger.info( - { limitCheck }, - `[EmailQueueService]: Limit check after sending email ${limitCheck.limit !== -1 && limitCheck.available !== undefined && !limitCheck.isLimitReached}` - ); + // Optional: remove or replace with structured fields + // logger.debug({ available: limitCheck.available, limit: limitCheck.limit }, "[EmailQueueService]: Post-send limit snapshot");apps/web/src/server/service/limit-service.ts (4)
138-146: Avoid logging full Team objectsLog identifiers/plan only to reduce PII and log volume.
- logger.info( - { dailyUsage, dailyLimit, team }, - `[LimitService]: Daily usage and limit` - ); + logger.info( + { teamId, plan: team.plan, dailyUsage, dailyLimit }, + "[LimitService]: Daily usage and limit" + );
158-164: Clamp available to 0 when limit reached (avoid negative values)Currently
availablecan be negative.return { isLimitReached: true, limit: dailyLimit, reason: LimitReason.EMAIL_DAILY_LIMIT_REACHED, - available: dailyLimit - dailyUsage, + available: 0, };
207-212: Same clamping for monthly limit reachedKeep
availablenon-negative.- available: monthlyLimit - monthlyUsage, + available: Math.max(0, monthlyLimit - monthlyUsage),
235-239: Handle unlimited (-1) cleanly and avoid negative available in normal pathExpose
availableonly when finite; otherwise undefined.- return { - isLimitReached: false, - limit: dailyLimit, - available: dailyLimit - dailyUsage, - }; + return { + isLimitReached: false, + limit: dailyLimit, + available: + dailyLimit === -1 ? undefined : Math.max(0, dailyLimit - dailyUsage), + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/src/server/mailer.ts(3 hunks)apps/web/src/server/service/email-queue-service.ts(4 hunks)apps/web/src/server/service/limit-service.ts(5 hunks)apps/web/src/server/service/team-service.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/server/service/limit-service.tsapps/web/src/server/mailer.tsapps/web/src/server/service/email-queue-service.tsapps/web/src/server/service/team-service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Prefer TypeScript for implementation files (TypeScript-first)
ESLint must pass with zero warnings using @usesend/eslint-config before PRs
Files:
apps/web/src/server/service/limit-service.tsapps/web/src/server/mailer.tsapps/web/src/server/service/email-queue-service.tsapps/web/src/server/service/team-service.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,md}: Use 2-space indentation and keep semicolons as enforced by Prettier 3
Run Prettier 3 formatting (pnpm format) over TypeScript and Markdown files
Files:
apps/web/src/server/service/limit-service.tsapps/web/src/server/mailer.tsapps/web/src/server/service/email-queue-service.tsapps/web/src/server/service/team-service.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
In apps/web, use the import alias "
/" for src imports (e.g., import { x } from "/utils/x")
Files:
apps/web/src/server/service/limit-service.tsapps/web/src/server/mailer.tsapps/web/src/server/service/email-queue-service.tsapps/web/src/server/service/team-service.ts
🧬 Code graph analysis (3)
apps/web/src/server/service/limit-service.ts (6)
apps/web/src/env.js (2)
env(5-125)env(5-125)apps/web/src/server/service/team-service.ts (1)
TeamService(17-502)apps/web/src/lib/constants/plans.ts (1)
PLAN_LIMITS(12-36)apps/web/src/server/redis.ts (1)
withCache(19-50)apps/web/src/server/service/usage-service.ts (1)
getThisMonthUsage(12-63)apps/web/src/server/logger/log.ts (1)
logger(31-63)
apps/web/src/server/service/email-queue-service.ts (2)
apps/web/src/server/logger/log.ts (1)
logger(31-63)apps/web/src/server/service/limit-service.ts (1)
LimitService(14-241)
apps/web/src/server/service/team-service.ts (8)
apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/redis.ts (1)
getRedis(6-13)apps/web/src/lib/constants/plans.ts (1)
PLAN_LIMITS(12-36)apps/web/src/server/public-api/api-error.ts (1)
UnsendApiError(62-75)apps/web/src/env.js (2)
env(5-125)env(5-125)apps/web/src/server/mailer.ts (1)
sendMail(72-135)apps/web/src/server/email-templates/UsageWarningEmail.tsx (1)
renderUsageWarningEmail(112-116)apps/web/src/server/email-templates/UsageLimitReachedEmail.tsx (1)
renderUsageLimitReachedEmail(106-110)
🔇 Additional comments (8)
apps/web/src/server/mailer.ts (1)
72-78: replyTo plumbed correctly in both providersParameter is optional and forwarded in self-hosted and UseSend paths. Looks good.
Also applies to: 104-112, 120-121
apps/web/src/server/service/email-queue-service.ts (1)
13-14: LGTM: centralizing limit checks via LimitServiceImport and inline usage keep queue path lean; aligns with PR objective to block sends on limits.
apps/web/src/server/service/team-service.ts (5)
83-97: LGTM: cache warm after team creationPre-populating cache avoids cold reads on first use.
103-110: LGTM: updateTeam refreshes cacheRefreshing after write-through keeps cache coherent.
242-256: LGTM: invalidate cache after role updateReasonable to drop cache on permission-affecting changes.
300-310: LGTM: invalidate cache after member removalKeeps downstream reads accurate.
504-522: LGTM: helper cleanly composes limit-reached email contentGood reuse and keeps render concerns isolated.
apps/web/src/server/service/limit-service.ts (1)
116-123: Requirement check: should unverified teams be blocked, or only flagged?The comment says “enforce verification and block flags,” but only
isBlockedis enforced. Confirm ifisVerified === falseshould also block (or reduce limits).Would you like a guarded block like:
if (!team.isVerified && team.plan !== "FREE") { return { isLimitReached: true, limit: 0, reason: LimitReason.EMAIL_BLOCKED }; }
1 participant
Summary by CodeRabbit
New Features
Improvements
Documentation