Conversation
|
Warning Rate limit exceeded@elie222 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 53 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 (12)
WalkthroughProvider-first refactor: Outlook webhook/history processing now consumes ParsedMessage and an EmailProvider; subscription management centralized into OutlookSubscriptionManager; watch scope broadened to all messages; Zod schemas made nullish; legacy per-account watch controller removed; per-notification clientState validation added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Notif as Outlook Notification
participant Webhook as /api/outlook/webhook
participant Orchestrator as processHistoryForUser
participant Provider as EmailProvider
participant ItemProc as processHistoryItem
participant SubsMgr as OutlookSubscriptionManager
Note right of Webhook: receives body.value[] (per-notification clientState checks)
Notif->>Webhook: POST notifications
Webhook->>Orchestrator: processHistoryForUser(resourceData...)
Orchestrator->>Provider: createEmailProvider(...) -- created once
Orchestrator->>ItemProc: processHistoryItem({ provider, resourceData, ... })
ItemProc->>ItemProc: read parsedMessage (headers, date, labels)
alt label ∉ {Inbox, Sent}
ItemProc-->>Orchestrator: skip (logged)
else Inbox or Sent
alt Sent (outbound)
ItemProc->>Provider: handleOutbound(parsedMessage, provider)
else Inbox (inbound)
ItemProc->>Provider: run blockers/categorization/rules with parsedMessage
end
ItemProc-->>Orchestrator: processed
end
Orchestrator->>SubsMgr: createManagedOutlookSubscription(emailAccountId)
SubsMgr->>Provider: watchEmails()
SubsMgr->>SubsMgr: cancel existing via unwatchEmails() if present
SubsMgr-->>Orchestrator: return expirationDate or null
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
apps/web/app/api/outlook/webhook/types.ts (1)
7-18: Remove unused accessToken from ProcessHistoryOptions and its call site
accessToken is declared in types.ts but never destructured or referenced in processHistoryItem—drop the field and the corresponding argument.
- apps/web/app/api/outlook/webhook/types.ts: remove
accessToken: string;- apps/web/app/api/outlook/webhook/process-history.ts: remove the
accessToken: emailAccount.account.access_token,lineapps/web/utils/outlook/watch.ts (1)
21-21: Broadened watch scope (/me/messages): validate event volume and filtering strategyThis will fire for all folders (including Junk/Deleted/Drafts). Ensure downstream filtering is correct and that queue throughput/limits are sized for the increased rate.
- Confirm expected notification rate after this change in staging.
- Verify that only intended changeTypes are required (e.g., do we need “updated” or would “created” suffice to reduce noise?).
apps/web/utils/outlook/message.ts (1)
80-94: Hoist FOLDER_TO_LABEL_MAP and tighten typesAvoid re-creating the map per call and strengthen key typing. Minor perf/readability gain.
- if (folderKey) { - const FOLDER_TO_LABEL_MAP = { - [WELL_KNOWN_FOLDERS.inbox]: OutlookLabel.INBOX, - [WELL_KNOWN_FOLDERS.sentitems]: OutlookLabel.SENT, - [WELL_KNOWN_FOLDERS.drafts]: OutlookLabel.DRAFT, - [WELL_KNOWN_FOLDERS.archive]: OutlookLabel.ARCHIVE, - [WELL_KNOWN_FOLDERS.junkemail]: OutlookLabel.SPAM, - [WELL_KNOWN_FOLDERS.deleteditems]: OutlookLabel.TRASH, - }; - - const label = - FOLDER_TO_LABEL_MAP[folderKey as keyof typeof FOLDER_TO_LABEL_MAP]; - if (label) { - labels.push(label); - } - } + if (folderKey) { + const label = FOLDER_TO_LABEL_MAP[folderKey as keyof typeof FOLDER_TO_LABEL_MAP]; + if (label) labels.push(label); + }Add near top of file (outside diff):
const FOLDER_TO_LABEL_MAP: Record<(typeof WELL_KNOWN_FOLDERS)[keyof typeof WELL_KNOWN_FOLDERS], string> = { [WELL_KNOWN_FOLDERS.inbox]: OutlookLabel.INBOX, [WELL_KNOWN_FOLDERS.sentitems]: OutlookLabel.SENT, [WELL_KNOWN_FOLDERS.drafts]: OutlookLabel.DRAFT, [WELL_KNOWN_FOLDERS.archive]: OutlookLabel.ARCHIVE, [WELL_KNOWN_FOLDERS.junkemail]: OutlookLabel.SPAM, [WELL_KNOWN_FOLDERS.deleteditems]: OutlookLabel.TRASH, };apps/web/app/api/outlook/webhook/process-history.ts (1)
71-75: Provider is created eagerly but also re-created later; pass it down onceCreate once here, pass into processHistoryItem, and remove the second creation inside process-history-item.ts to save work and reduce latency.
Example (outside this diff):
- Update ProcessHistoryOptions to include provider
- Pass provider here
- Remove createEmailProvider in process-history-item.ts
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/web/app/api/outlook/webhook/process-history-item.ts(4 hunks)apps/web/app/api/outlook/webhook/process-history.ts(2 hunks)apps/web/app/api/outlook/webhook/types.ts(1 hunks)apps/web/utils/outlook/message.ts(1 hunks)apps/web/utils/outlook/watch.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/utils/outlook/watch.tsapps/web/utils/outlook/message.tsapps/web/app/api/outlook/webhook/process-history.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/types.ts
!{.cursor/rules/*.mdc}
📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/utils/outlook/watch.tsapps/web/utils/outlook/message.tsapps/web/app/api/outlook/webhook/process-history.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/types.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod
Files:
apps/web/utils/outlook/watch.tsapps/web/utils/outlook/message.tsapps/web/app/api/outlook/webhook/process-history.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/types.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/utils/outlook/watch.tsapps/web/utils/outlook/message.tsapps/web/app/api/outlook/webhook/process-history.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/types.ts
apps/web/utils/**
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Create utility functions in
utils/folder for reusable logic
Files:
apps/web/utils/outlook/watch.tsapps/web/utils/outlook/message.ts
apps/web/utils/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size
Files:
apps/web/utils/outlook/watch.tsapps/web/utils/outlook/message.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/utils/outlook/watch.tsapps/web/utils/outlook/message.tsapps/web/app/api/outlook/webhook/process-history.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/types.ts
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/utils/outlook/watch.tsapps/web/utils/outlook/message.tsapps/web/app/api/outlook/webhook/process-history.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/types.ts
apps/web/app/**
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
NextJS app router structure with (app) directory
Files:
apps/web/app/api/outlook/webhook/process-history.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/types.ts
apps/web/app/api/**/*.{ts,js}
📄 CodeRabbit inference engine (.cursor/rules/security-audit.mdc)
apps/web/app/api/**/*.{ts,js}: All API route handlers in 'apps/web/app/api/' must use authentication middleware: withAuth, withEmailAccount, or withError (with custom authentication logic).
All Prisma queries in API routes must include user/account filtering (e.g., emailAccountId or userId in WHERE clauses) to prevent unauthorized data access.
All parameters used in API routes must be validated before use; do not use parameters from 'params' or request bodies directly in queries without validation.
Request bodies in API routes should use Zod schemas for validation.
API routes should only return necessary fields using Prisma's 'select' and must not include sensitive data in error messages.
Error messages in API routes must not reveal internal details; use generic errors and SafeError for user-facing errors.
All QStash endpoints (API routes called via publishToQstash or publishToQstashQueue) must use verifySignatureAppRouter to verify request authenticity.
All cron endpoints in API routes must use hasCronSecret or hasPostCronSecret for authentication.
Do not hardcode weak or plaintext secrets in API route files; secrets must not be directly assigned as string literals.
Review all new withError usage in API routes to ensure custom authentication is implemented where required.
Files:
apps/web/app/api/outlook/webhook/process-history.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/types.ts
🧬 Code graph analysis (4)
apps/web/utils/outlook/message.ts (1)
apps/web/utils/outlook/label.ts (1)
OutlookLabel(16-26)
apps/web/app/api/outlook/webhook/process-history.ts (1)
apps/web/utils/email/provider.ts (1)
createEmailProvider(13-28)
apps/web/app/api/outlook/webhook/process-history-item.ts (2)
apps/web/utils/outlook/message.ts (1)
WELL_KNOWN_FOLDERS(14-21)apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)
apps/web/app/api/outlook/webhook/types.ts (3)
apps/web/utils/outlook/client.ts (1)
Client(173-173)apps/web/utils/types.ts (1)
RuleWithActionsAndCategories(16-18)apps/web/utils/llms/types.ts (1)
EmailAccountWithAI(10-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Static Code Analysis Js
- GitHub Check: Secret Detection
- GitHub Check: Jit Security
🔇 Additional comments (3)
apps/web/app/api/outlook/webhook/types.ts (1)
45-45: LGTM: zod-inferred OutlookResourceDataGood move to a single source of truth for types via Zod inference.
apps/web/app/api/outlook/webhook/process-history-item.ts (2)
151-152: LGTM: outbound detection switched to Sent ItemsOnce the folder ID comparison is fixed, this is the right signal for outbound.
271-277: Accepting nullable conversationId is correctOutlook may omit conversationId; the updated signature avoids non-null assertions.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/utils/email/microsoft.ts (3)
876-899: Type mismatch: conversationId can be null but local type is string | undefinedprocessHistory() sets conversationId to null, but the inline type only allows string | undefined. Align with the shared schema to avoid TS errors and future drift.
Apply one of the following:
Option A — reuse shared type
+import type { OutlookResourceData } from "@/app/api/outlook/webhook/types"; ... - async processHistory(options: { + async processHistory(options: { emailAddress: string; historyId?: number; startHistoryId?: number; subscriptionId?: string; - resourceData?: { - id: string; - conversationId?: string; - }; + resourceData?: OutlookResourceData; }): Promise<void> { ... resourceData: options.resourceData || { id: options.historyId?.toString() || "0", - conversationId: options.startHistoryId?.toString() || null, + conversationId: options.startHistoryId?.toString() ?? null, },Option B — relax the local type
- resourceData?: { - id: string; - conversationId?: string; - }; + resourceData?: { + id: string; + conversationId?: string | null; + };
346-355: Inbox filter uses literal 'inbox' instead of real folder idparentFolderId eq 'inbox' will not match; parentFolderId is a GUID. Fetch the real Inbox id via getFolderIds() or query the inbox endpoint.
- const response = await client - .api("/me/messages") - .filter(`conversationId eq '${threadId}' and parentFolderId eq 'inbox'`) + const { inbox: inboxId } = await getFolderIds(this.client); + if (!inboxId) throw new Error("Inbox folder id not found"); + const response = await client + .api("/me/messages") + .filter(`conversationId eq '${threadId}' and parentFolderId eq '${inboxId}'`)
639-653: Folder filtering for “all”/labelId also uses literal namesparentFolderId eq 'inbox'/'archive' and treating labelId as a folder slug are brittle. Resolve actual folder IDs first.
- if (query?.type === "sent") { - endpoint = "/me/mailFolders('sentitems')/messages"; - } else if (query?.type === "all") { - filters.push("(parentFolderId eq 'inbox' or parentFolderId eq 'archive')"); - } else if (query?.labelId) { - filters.push(`parentFolderId eq '${query.labelId.toLowerCase()}'`); + const folderIds = await getFolderIds(this.client); + if (query?.type === "sent") { + endpoint = "/me/mailFolders('sentitems')/messages"; + } else if (query?.type === "all") { + const inbox = folderIds.inbox; + const archive = folderIds.archive; + filters.push(`(parentFolderId eq '${inbox}' or parentFolderId eq '${archive}')`); + } else if (query?.labelId) { + // If labelId is a known slug, map it; otherwise treat it as a real folder id. + const mapped = + folderIds[query.labelId.toLowerCase() as keyof typeof folderIds] ?? + query.labelId; + filters.push(`parentFolderId eq '${mapped}'`);apps/web/app/api/outlook/watch/all/route.ts (1)
75-81: Use the primary key for the update filterReplace the email-based filter with the PK in apps/web/app/api/outlook/watch/all/route.ts to avoid relying on natural keys:
- await prisma.emailAccount.update({ - where: { email: emailAccount.email }, + await prisma.emailAccount.update({ + where: { id: emailAccount.id }, data: { watchEmailsExpirationDate: null, watchEmailsSubscriptionId: null, }, });Email is already marked
@uniquein apps/web/prisma/schema.prisma:99, butidremains the most stable identifier.
♻️ Duplicate comments (1)
apps/web/app/api/outlook/webhook/types.ts (1)
28-36: Make resourceData optional+nullable to accept Graph payloads without itGraph can omit resourceData depending on subscription/config. Keeping it required will reject valid notifications.
Apply this diff for consistency with your nullish usage:
- resourceData: resourceDataSchema, + resourceData: resourceDataSchema.nullish(),
🧹 Nitpick comments (12)
apps/web/utils/email/microsoft.ts (1)
925-935: Use Buffer decoding instead of atob in Node environmentatob may not exist server-side and will spuriously warn. Use Buffer to detect replies by conversationIndex length.
- isReplyInThread(message: ParsedMessage): boolean { - try { - return atob(message.conversationIndex || "").length > 22; + isReplyInThread(message: ParsedMessage): boolean { + try { + const raw = message.conversationIndex ?? ""; + const len = + raw ? Buffer.from(raw, "base64").toString("binary").length : 0; + return len > 22;apps/web/utils/outlook/subscription-manager.ts (4)
14-20: Narrow the dependency type and name it provider for clarityAvoid confusion with Outlook “client” types and enforce only the methods you need.
-export class OutlookSubscriptionManager { - private readonly client: EmailProvider; +export class OutlookSubscriptionManager { + private readonly provider: Pick<EmailProvider, "watchEmails" | "unwatchEmails">; private readonly emailAccountId: string; - constructor(client: EmailProvider, emailAccountId: string) { - this.client = client; + constructor(provider: Pick<EmailProvider, "watchEmails" | "unwatchEmails">, emailAccountId: string) { + this.provider = provider; this.emailAccountId = emailAccountId; }
22-39: Add explicit return type to createSubscriptionImproves DX and prevents accidental shape drift.
- async createSubscription() { + async createSubscription(): Promise<{ subscriptionId?: string; expirationDate: Date } | null> { ... - const subscription = await this.client.watchEmails(); + const subscription = await this.provider.watchEmails();
59-76: Update property rename in unwatch pathFollow-through on provider rename.
- await this.client.unwatchEmails(existingSubscriptionId); + await this.provider.unwatchEmails(existingSubscriptionId);
125-143: Specify helper return type and ensure provider instance is passedMinor type clarity; no behavior change.
-export async function createManagedOutlookSubscription(emailAccountId: string) { +export async function createManagedOutlookSubscription(emailAccountId: string): Promise<Date | null> { const provider = await createEmailProvider({ emailAccountId, provider: "microsoft", }); - const manager = new OutlookSubscriptionManager(provider, emailAccountId); + const manager = new OutlookSubscriptionManager(provider, emailAccountId);apps/web/__tests__/outlook-subscription-manager.test.ts (2)
7-15: Prefer the shared Prisma mock utilityAdopt the project’s standard Prisma mock to reduce duplication.
-vi.mock("@/utils/prisma", () => ({ - default: { - emailAccount: { - findUnique: vi.fn(), - update: vi.fn(), - }, - }, -})); +vi.mock("@/utils/prisma"); +// Optionally: import { prisma } from "@/utils/__mocks__/prisma" if exposed
1-6: Remove unused Graph Client importNot needed after switching to provider stub.
-import type { Client } from "@microsoft/microsoft-graph-client";apps/web/app/api/outlook/webhook/types.ts (1)
20-26: Optional: tighten validation for changeType/dateConsider stricter validation to catch bad payloads early.
- changeType: z.string(), + changeType: z.enum(["created","updated","deleted","moved","missed"]), // adjust to your supported set- subscriptionExpirationDateTime: z.string().nullish(), + subscriptionExpirationDateTime: z.string().datetime().nullish(),apps/web/app/api/outlook/watch/all/route.ts (2)
87-95: Optional: clear stale watch data when tokens are missingIf tokens are gone and the watch is expired, clear watch fields like you do for non-AI users.
if ( - !emailAccount.account?.access_token || - !emailAccount.account?.refresh_token + !emailAccount.account?.access_token || + !emailAccount.account?.refresh_token ) { logger.info("User has no access token or refresh token", { email: emailAccount.email, }); + if ( + emailAccount.watchEmailsExpirationDate && + new Date(emailAccount.watchEmailsExpirationDate) < new Date() + ) { + await prisma.emailAccount.update({ + where: { id: emailAccount.id }, + data: { watchEmailsExpirationDate: null, watchEmailsSubscriptionId: null }, + }); + } continue; }
55-86: Optional: add gentle concurrency to speed up cron without overloading GraphProcess accounts with a small parallelism limit (e.g., 3–5) to reduce total runtime.
apps/web/app/api/outlook/watch/route.ts (2)
15-23: Avoid N+1: fetch token presence in the initial queryYou can select token fields with the first findMany and drop the per-account findUnique.
- const emailAccounts = await prisma.emailAccount.findMany({ + const emailAccounts = await prisma.emailAccount.findMany({ where: { userId, account: { provider: "microsoft", }, }, - select: { id: true }, + select: { + id: true, + account: { select: { access_token: true, refresh_token: true } }, + }, }); @@ - const account = await prisma.emailAccount.findUnique({ - where: { id: emailAccountId }, - select: { - account: { - select: { - access_token: true, - refresh_token: true, - expires_at: true, - }, - }, - }, - }); + const account = emailAccounts.find(a => a.id === emailAccountId);Also applies to: 34-45
1-11: Optional: export a response type for GETFor type-safe client usage per guidelines.
export type OutlookWatchResponse = { results: Array< | { emailAccountId: string; status: "success"; expirationDate: Date } | { emailAccountId: string; status: "error"; message: string; errorDetails?: string } >; };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
apps/web/__tests__/outlook-subscription-manager.test.ts(1 hunks)apps/web/app/api/outlook/watch/all/route.ts(2 hunks)apps/web/app/api/outlook/watch/controller.ts(0 hunks)apps/web/app/api/outlook/watch/route.ts(2 hunks)apps/web/app/api/outlook/webhook/types.ts(1 hunks)apps/web/app/api/watch/controller.ts(2 hunks)apps/web/utils/email/microsoft.ts(2 hunks)apps/web/utils/outlook/subscription-manager.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/app/api/outlook/watch/controller.ts
🧰 Additional context used
📓 Path-based instructions (19)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/__tests__/outlook-subscription-manager.test.tsapps/web/utils/email/microsoft.tsapps/web/utils/outlook/subscription-manager.tsapps/web/app/api/outlook/watch/route.tsapps/web/app/api/outlook/watch/all/route.tsapps/web/app/api/watch/controller.tsapps/web/app/api/outlook/webhook/types.ts
!{.cursor/rules/*.mdc}
📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/__tests__/outlook-subscription-manager.test.tsapps/web/utils/email/microsoft.tsapps/web/utils/outlook/subscription-manager.tsapps/web/app/api/outlook/watch/route.tsapps/web/app/api/outlook/watch/all/route.tsapps/web/app/api/watch/controller.tsapps/web/app/api/outlook/webhook/types.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod
Files:
apps/web/__tests__/outlook-subscription-manager.test.tsapps/web/utils/email/microsoft.tsapps/web/utils/outlook/subscription-manager.tsapps/web/app/api/outlook/watch/route.tsapps/web/app/api/outlook/watch/all/route.tsapps/web/app/api/watch/controller.tsapps/web/app/api/outlook/webhook/types.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/__tests__/outlook-subscription-manager.test.tsapps/web/utils/email/microsoft.tsapps/web/utils/outlook/subscription-manager.tsapps/web/app/api/outlook/watch/route.tsapps/web/app/api/outlook/watch/all/route.tsapps/web/app/api/watch/controller.tsapps/web/app/api/outlook/webhook/types.ts
**/*.test.{ts,js}
📄 CodeRabbit inference engine (.cursor/rules/security.mdc)
Include security tests in your test suites to verify authentication, authorization, and error handling.
Files:
apps/web/__tests__/outlook-subscription-manager.test.ts
**/*.test.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
**/*.test.{ts,js,tsx,jsx}: Tests are colocated next to the tested file (e.g.,dir/format.tsanddir/format.test.ts)
Usevi.mock("server-only", () => ({}));to mock theserver-onlymodule in tests
Mock@/utils/prismain tests usingvi.mock("@/utils/prisma")and use the provided prisma mock
Mock external dependencies in tests
Clean up mocks between tests
Do not mock the Logger
Files:
apps/web/__tests__/outlook-subscription-manager.test.ts
**/__tests__/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
AI tests are placed in the
__tests__directory and are not run by default (they use a real LLM)
Files:
apps/web/__tests__/outlook-subscription-manager.test.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/__tests__/outlook-subscription-manager.test.tsapps/web/utils/email/microsoft.tsapps/web/utils/outlook/subscription-manager.tsapps/web/app/api/outlook/watch/route.tsapps/web/app/api/outlook/watch/all/route.tsapps/web/app/api/watch/controller.tsapps/web/app/api/outlook/webhook/types.ts
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/__tests__/outlook-subscription-manager.test.tsapps/web/utils/email/microsoft.tsapps/web/utils/outlook/subscription-manager.tsapps/web/app/api/outlook/watch/route.tsapps/web/app/api/outlook/watch/all/route.tsapps/web/app/api/watch/controller.tsapps/web/app/api/outlook/webhook/types.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Don't use export or module.exports in test files.
Don't use focused tests.
Don't use disabled tests.
Make sure the assertion function, like expect, is placed inside an it() function call.
Don't nest describe() blocks too deeply in test files.
Don't use focused tests.
Don't use disabled tests.
Don't use export or module.exports in test files.
Files:
apps/web/__tests__/outlook-subscription-manager.test.ts
apps/web/__tests__/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm.mdc)
Place LLM-specific tests under apps/web/tests
Files:
apps/web/__tests__/outlook-subscription-manager.test.ts
apps/web/__tests__/**
📄 CodeRabbit inference engine (.cursor/rules/llm-test.mdc)
Place all LLM-related tests under apps/web/tests/
Files:
apps/web/__tests__/outlook-subscription-manager.test.ts
apps/web/__tests__/**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-test.mdc)
apps/web/__tests__/**/*.test.ts: Guard LLM tests with describe.runIf(process.env.RUN_AI_TESTS === "true") so they only run when RUN_AI_TESTS="true"
Mock the "server-only" module in LLM tests with vi.mock("server-only", () => ({}))
Clear mocks in a beforeEach using vi.clearAllMocks()
Set and use a generous timeout (e.g., const TIMEOUT = 15_000) for LLM tests and pass it to long-running tests/describe
Create helper functions for common test data (e.g., getUser, getTestData(overrides))
Include standard cases: happy path, error handling, edge cases (empty/null), different user configs, and various input formats
Use console.debug to log generated content for inspection
Do not mock the LLM call; invoke the real LLM in these tests
Test both AI and non-AI code paths (e.g., return unchanged when no AI processing is needed)
Files:
apps/web/__tests__/outlook-subscription-manager.test.ts
apps/web/utils/**
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Create utility functions in
utils/folder for reusable logic
Files:
apps/web/utils/email/microsoft.tsapps/web/utils/outlook/subscription-manager.ts
apps/web/utils/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size
Files:
apps/web/utils/email/microsoft.tsapps/web/utils/outlook/subscription-manager.ts
apps/web/app/**
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
NextJS app router structure with (app) directory
Files:
apps/web/app/api/outlook/watch/route.tsapps/web/app/api/outlook/watch/all/route.tsapps/web/app/api/watch/controller.tsapps/web/app/api/outlook/webhook/types.ts
apps/web/app/api/**/route.ts
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/app/api/**/route.ts: UsewithAuthfor user-level operations
UsewithEmailAccountfor email-account-level operations
Do NOT use POST API routes for mutations - use server actions instead
No need for try/catch in GET routes when using middleware
Export response types from GET routes
apps/web/app/api/**/route.ts: Wrap all GET API route handlers withwithAuthorwithEmailAccountmiddleware for authentication and authorization.
Export response types from GET API routes for type-safe client usage.
Do not use try/catch in GET API routes when using authentication middleware; rely on centralized error handling.
Files:
apps/web/app/api/outlook/watch/route.tsapps/web/app/api/outlook/watch/all/route.ts
**/api/**/route.ts
📄 CodeRabbit inference engine (.cursor/rules/security.mdc)
**/api/**/route.ts: ALL API routes that handle user data MUST use appropriate authentication and authorization middleware (withAuth or withEmailAccount).
ALL database queries in API routes MUST be scoped to the authenticated user/account (e.g., include userId or emailAccountId in query filters).
Always validate that resources belong to the authenticated user before performing operations (resource ownership validation).
UsewithEmailAccountmiddleware for API routes that operate on a specific email account (i.e., use or requireemailAccountId).
UsewithAuthmiddleware for API routes that operate at the user level (i.e., use or require onlyuserId).
UsewithErrormiddleware (with proper validation) for public endpoints, custom authentication, or cron endpoints.
Cron endpoints MUST usewithErrormiddleware and validate the cron secret usinghasCronSecret(request)orhasPostCronSecret(request).
Cron endpoints MUST capture unauthorized attempts withcaptureExceptionand return a 401 status for unauthorized requests.
All parameters in API routes MUST be validated for type, format, and length before use.
Request bodies in API routes MUST be validated using Zod schemas before use.
All Prisma queries in API routes MUST only return necessary fields and never expose sensitive data.
Error messages in API routes MUST not leak internal information or sensitive data; use generic error messages and SafeError where appropriate.
API routes MUST use a consistent error response format, returning JSON with an error message and status code.
AllfindUniqueandfindFirstPrisma calls in API routes MUST include ownership filters (e.g., userId or emailAccountId).
AllfindManyPrisma calls in API routes MUST be scoped to the authenticated user's data.
Never use direct object references in API routes without ownership checks (prevent IDOR vulnerabilities).
Prevent mass assignment vulnerabilities by only allowing explicitly whitelisted fields in update operations in AP...
Files:
apps/web/app/api/outlook/watch/route.tsapps/web/app/api/outlook/watch/all/route.ts
apps/web/app/api/**/*.{ts,js}
📄 CodeRabbit inference engine (.cursor/rules/security-audit.mdc)
apps/web/app/api/**/*.{ts,js}: All API route handlers in 'apps/web/app/api/' must use authentication middleware: withAuth, withEmailAccount, or withError (with custom authentication logic).
All Prisma queries in API routes must include user/account filtering (e.g., emailAccountId or userId in WHERE clauses) to prevent unauthorized data access.
All parameters used in API routes must be validated before use; do not use parameters from 'params' or request bodies directly in queries without validation.
Request bodies in API routes should use Zod schemas for validation.
API routes should only return necessary fields using Prisma's 'select' and must not include sensitive data in error messages.
Error messages in API routes must not reveal internal details; use generic errors and SafeError for user-facing errors.
All QStash endpoints (API routes called via publishToQstash or publishToQstashQueue) must use verifySignatureAppRouter to verify request authenticity.
All cron endpoints in API routes must use hasCronSecret or hasPostCronSecret for authentication.
Do not hardcode weak or plaintext secrets in API route files; secrets must not be directly assigned as string literals.
Review all new withError usage in API routes to ensure custom authentication is implemented where required.
Files:
apps/web/app/api/outlook/watch/route.tsapps/web/app/api/outlook/watch/all/route.tsapps/web/app/api/watch/controller.tsapps/web/app/api/outlook/webhook/types.ts
🧠 Learnings (3)
📚 Learning: 2025-08-31T12:17:19.408Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-08-31T12:17:19.408Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Include standard cases: happy path, error handling, edge cases (empty/null), different user configs, and various input formats
Applied to files:
apps/web/__tests__/outlook-subscription-manager.test.ts
📚 Learning: 2025-06-23T12:27:30.570Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-06-23T12:27:30.570Z
Learning: When mocking Prisma in Vitest, import the Prisma mock from '@/utils/__mocks__/prisma', mock '@/utils/prisma', and clear all mocks in a beforeEach hook to ensure test isolation.
Applied to files:
apps/web/__tests__/outlook-subscription-manager.test.ts
📚 Learning: 2025-07-18T15:04:30.467Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-07-18T15:04:30.467Z
Learning: Applies to apps/web/app/api/**/route.ts : Use `withEmailAccount` for email-account-level operations
Applied to files:
apps/web/app/api/outlook/watch/all/route.ts
🧬 Code graph analysis (6)
apps/web/__tests__/outlook-subscription-manager.test.ts (2)
apps/web/utils/outlook/subscription-manager.ts (1)
OutlookSubscriptionManager(13-123)apps/web/utils/outlook/watch.ts (2)
watchOutlook(6-34)unwatchOutlook(36-38)
apps/web/utils/outlook/subscription-manager.ts (3)
apps/web/utils/logger.ts (1)
createScopedLogger(21-70)apps/web/utils/error.ts (1)
captureException(63-75)apps/web/utils/email/provider.ts (1)
createEmailProvider(13-28)
apps/web/app/api/outlook/watch/route.ts (1)
apps/web/utils/outlook/subscription-manager.ts (1)
createManagedOutlookSubscription(125-143)
apps/web/app/api/outlook/watch/all/route.ts (1)
apps/web/utils/outlook/subscription-manager.ts (1)
createManagedOutlookSubscription(125-143)
apps/web/app/api/watch/controller.ts (2)
apps/web/utils/email/provider-types.ts (1)
isMicrosoftProvider(5-7)apps/web/utils/outlook/subscription-manager.ts (1)
createManagedOutlookSubscription(125-143)
apps/web/app/api/outlook/webhook/types.ts (3)
apps/web/utils/outlook/client.ts (1)
Client(173-173)apps/web/utils/types.ts (1)
RuleWithActionsAndCategories(16-18)apps/web/utils/llms/types.ts (1)
EmailAccountWithAI(10-29)
🪛 GitHub Actions: Run Tests
apps/web/__tests__/outlook-subscription-manager.test.ts
[error] 1-1: pnpm -F inbox-zero-ai test failed due to server/client boundary error: 'This module cannot be imported from a Client Component module. It should only be used from a Server Component.'
🔇 Additional comments (5)
apps/web/utils/email/microsoft.ts (1)
59-59: No-op import reorderNo action needed.
apps/web/app/api/outlook/webhook/types.ts (2)
7-18: ProcessHistoryOptions: type shape and placement look goodExported where it’s used and composed from existing types; no issues.
31-35: Good fix on nullability: use of .nullish() avoids parse failuresSwitching resource, subscriptionExpirationDateTime, clientState, and tenantId to nullish aligns with Graph’s variability.
apps/web/app/api/outlook/watch/route.ts (1)
57-58: Delegation to subscription manager: LGTMCentralizing Outlook subscription creation reduces duplication.
apps/web/app/api/watch/controller.ts (1)
41-48: Good: unified no-result path with structured loggingClearer failure semantics if provider returns a falsy result.
| vi.mock("@/utils/logger", () => ({ | ||
| createScopedLogger: () => ({ | ||
| info: vi.fn(), | ||
| warn: vi.fn(), | ||
| error: vi.fn(), | ||
| }), | ||
| })); | ||
|
|
||
| vi.mock("@/utils/error", () => ({ | ||
| captureException: vi.fn(), | ||
| })); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not mock the Logger per repo guidelines
Remove the logger mock; let the real scoped logger output.
-vi.mock("@/utils/logger", () => ({
- createScopedLogger: () => ({
- info: vi.fn(),
- warn: vi.fn(),
- error: vi.fn(),
- }),
-}));📝 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.
| vi.mock("@/utils/logger", () => ({ | |
| createScopedLogger: () => ({ | |
| info: vi.fn(), | |
| warn: vi.fn(), | |
| error: vi.fn(), | |
| }), | |
| })); | |
| vi.mock("@/utils/error", () => ({ | |
| captureException: vi.fn(), | |
| })); | |
| // Removed the logger mock per repo guidelines to allow real scoped logger output | |
| vi.mock("@/utils/error", () => ({ | |
| captureException: vi.fn(), | |
| })); |
🤖 Prompt for AI Agents
In apps/web/__tests__/outlook-subscription-manager.test.ts around lines 22 to
33, remove the vi.mock block that mocks "@/utils/logger" so the test uses the
real scoped logger per repo guidelines; delete the entire
vi.mock("@/utils/logger", ...) block and update any test assertions that
depended on the mocked logger (e.g., remove or replace expectations against
info/warn/error mocks) while leaving the "@/utils/error" mock intact.
| it("should create subscription even if no existing subscription exists", async () => { | ||
| // Mock no existing subscription | ||
| vi.mocked(prisma.emailAccount.findUnique).mockResolvedValue({ | ||
| watchEmailsSubscriptionId: null, | ||
| } as any); | ||
|
|
||
| const newSubscription = { | ||
| id: "new-subscription-id", | ||
| expirationDateTime: new Date().toISOString(), | ||
| }; | ||
| vi.mocked(watchOutlook).mockResolvedValue(newSubscription); | ||
|
|
||
| // Act | ||
| const result = await manager.createSubscription(); | ||
|
|
||
| // Assert | ||
| expect(unwatchOutlook).not.toHaveBeenCalled(); | ||
| expect(watchOutlook).toHaveBeenCalledWith(mockClient); | ||
| expect(result).toEqual(newSubscription); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same fixes for “no existing subscription” path
- const newSubscription = {
- id: "new-subscription-id",
- expirationDateTime: new Date().toISOString(),
- };
- vi.mocked(watchOutlook).mockResolvedValue(newSubscription);
+ const newSubscription = {
+ subscriptionId: "new-subscription-id",
+ expirationDate: new Date(),
+ };
+ vi.mocked(provider.watchEmails).mockResolvedValue(newSubscription as any);
...
- expect(unwatchOutlook).not.toHaveBeenCalled();
- expect(watchOutlook).toHaveBeenCalledWith(mockClient);
+ expect(provider.unwatchEmails).not.toHaveBeenCalled();
+ expect(provider.watchEmails).toHaveBeenCalled();
expect(result).toEqual(newSubscription);📝 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.
| it("should create subscription even if no existing subscription exists", async () => { | |
| // Mock no existing subscription | |
| vi.mocked(prisma.emailAccount.findUnique).mockResolvedValue({ | |
| watchEmailsSubscriptionId: null, | |
| } as any); | |
| const newSubscription = { | |
| id: "new-subscription-id", | |
| expirationDateTime: new Date().toISOString(), | |
| }; | |
| vi.mocked(watchOutlook).mockResolvedValue(newSubscription); | |
| // Act | |
| const result = await manager.createSubscription(); | |
| // Assert | |
| expect(unwatchOutlook).not.toHaveBeenCalled(); | |
| expect(watchOutlook).toHaveBeenCalledWith(mockClient); | |
| expect(result).toEqual(newSubscription); | |
| }); | |
| it("should create subscription even if no existing subscription exists", async () => { | |
| // Mock no existing subscription | |
| vi.mocked(prisma.emailAccount.findUnique).mockResolvedValue({ | |
| watchEmailsSubscriptionId: null, | |
| } as any); | |
| const newSubscription = { | |
| subscriptionId: "new-subscription-id", | |
| expirationDate: new Date(), | |
| }; | |
| vi.mocked(provider.watchEmails).mockResolvedValue(newSubscription as any); | |
| // Act | |
| const result = await manager.createSubscription(); | |
| // Assert | |
| expect(provider.unwatchEmails).not.toHaveBeenCalled(); | |
| expect(provider.watchEmails).toHaveBeenCalled(); | |
| expect(result).toEqual(newSubscription); | |
| }); |
🤖 Prompt for AI Agents
In apps/web/__tests__/outlook-subscription-manager.test.ts around lines 72 to
91, apply the same fixes as the other "no existing subscription" test: have
prisma.emailAccount.findUnique return null
(vi.mocked(prisma.emailAccount.findUnique).mockResolvedValue(null)) instead of
an object with watchEmailsSubscriptionId: null, keep watchOutlook mocked to
return newSubscription, and assert unwatchOutlook was not called, watchOutlook
was called with mockClient, and the result equals newSubscription; update the
mock typings if needed to satisfy TypeScript.
| it("should return null if creating new subscription fails", async () => { | ||
| // Mock no existing subscription | ||
| vi.mocked(prisma.emailAccount.findUnique).mockResolvedValue({ | ||
| watchEmailsSubscriptionId: null, | ||
| } as any); | ||
|
|
||
| // Mock watchOutlook to fail | ||
| vi.mocked(watchOutlook).mockRejectedValue(new Error("API error")); | ||
|
|
||
| // Act | ||
| const result = await manager.createSubscription(); | ||
|
|
||
| // Assert | ||
| expect(result).toBeNull(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Error path: make watchEmails reject and expect null
- vi.mocked(watchOutlook).mockRejectedValue(new Error("API error"));
+ vi.mocked(provider.watchEmails).mockRejectedValue(new Error("API error"));📝 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.
| it("should return null if creating new subscription fails", async () => { | |
| // Mock no existing subscription | |
| vi.mocked(prisma.emailAccount.findUnique).mockResolvedValue({ | |
| watchEmailsSubscriptionId: null, | |
| } as any); | |
| // Mock watchOutlook to fail | |
| vi.mocked(watchOutlook).mockRejectedValue(new Error("API error")); | |
| // Act | |
| const result = await manager.createSubscription(); | |
| // Assert | |
| expect(result).toBeNull(); | |
| }); | |
| }); | |
| it("should return null if creating new subscription fails", async () => { | |
| // Mock no existing subscription | |
| vi.mocked(prisma.emailAccount.findUnique).mockResolvedValue({ | |
| watchEmailsSubscriptionId: null, | |
| } as any); | |
| // Mock watchOutlook to fail | |
| vi.mocked(provider.watchEmails).mockRejectedValue(new Error("API error")); | |
| // Act | |
| const result = await manager.createSubscription(); | |
| // Assert | |
| expect(result).toBeNull(); | |
| }); |
🤖 Prompt for AI Agents
In apps/web/__tests__/outlook-subscription-manager.test.ts around lines 119 to
134, the test intends to simulate the error path by making the watchEmails call
reject but currently mocks watchOutlook to reject instead; update the test to
mock the actual function used by createSubscription (replace
vi.mocked(watchOutlook).mockRejectedValue(...) with
vi.mocked(watchEmails).mockRejectedValue(new Error("API error")) or mock the
correct exported function name), keep the existing prisma mock, call
manager.createSubscription, and assert it returns null so the test matches the
intended error path.
| it("should throw error if subscription has no expiration date", async () => { | ||
| const subscription = { | ||
| id: "test-subscription-id", | ||
| expirationDateTime: undefined, | ||
| }; | ||
|
|
||
| // Act & Assert | ||
| await expect( | ||
| manager.updateSubscriptionInDatabase(subscription), | ||
| ).rejects.toThrow("Subscription missing expiration date"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
updateSubscriptionInDatabase: missing expirationDate check should test current contract
- const subscription = {
- id: "test-subscription-id",
- expirationDateTime: undefined,
- };
+ const subscription = {
+ subscriptionId: "test-subscription-id",
+ // @ts-expect-error testing runtime guard
+ expirationDate: undefined,
+ };📝 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.
| it("should throw error if subscription has no expiration date", async () => { | |
| const subscription = { | |
| id: "test-subscription-id", | |
| expirationDateTime: undefined, | |
| }; | |
| // Act & Assert | |
| await expect( | |
| manager.updateSubscriptionInDatabase(subscription), | |
| ).rejects.toThrow("Subscription missing expiration date"); | |
| }); | |
| }); | |
| it("should throw error if subscription has no expiration date", async () => { | |
| const subscription = { | |
| subscriptionId: "test-subscription-id", | |
| // @ts-expect-error testing runtime guard | |
| expirationDate: undefined, | |
| }; | |
| // Act & Assert | |
| await expect( | |
| manager.updateSubscriptionInDatabase(subscription), | |
| ).rejects.toThrow("Subscription missing expiration date"); | |
| }); |
🤖 Prompt for AI Agents
In apps/web/__tests__/outlook-subscription-manager.test.ts around lines 156-167,
the test constructs a subscription using expirationDateTime but the current
subscription contract expects expirationDate (and may return null/undefined
differently); update the test to use the current contract shape (use
expirationDate property or omit it entirely to simulate a missing date), and
assert the error message that the real function throws (adjust the toThrow
string to match the actual thrown message) so the test reflects the current
contract and runtime behavior.
| const result = await provider.watchEmails(); | ||
|
|
||
| if (result) { | ||
| await prisma.emailAccount.update({ | ||
| where: { id: emailAccountId }, | ||
| data: { watchEmailsExpirationDate: result.expirationDate }, | ||
| }); | ||
| return { success: true, expirationDate: result.expirationDate }; | ||
| } |
There was a problem hiding this comment.
Bug: non-Microsoft path drops subscriptionId; unwatch will lack the ID
You no longer persist watchEmailsSubscriptionId for non-Microsoft providers, but unwatchEmails still expects it from DB. This breaks unwatch and renewal flows.
} else {
const result = await provider.watchEmails();
if (result) {
- await prisma.emailAccount.update({
+ await prisma.emailAccount.update({
where: { id: emailAccountId },
- data: { watchEmailsExpirationDate: result.expirationDate },
+ data: {
+ watchEmailsExpirationDate: result.expirationDate,
+ watchEmailsSubscriptionId: result.subscriptionId ?? null,
+ },
});
return { success: true, expirationDate: result.expirationDate };
}
}🤖 Prompt for AI Agents
In apps/web/app/api/watch/controller.ts around lines 30-38, the code updates
only watchEmailsExpirationDate after provider.watchEmails(), which drops
watchEmailsSubscriptionId for non-Microsoft providers and breaks unwatch/renew
flows; modify the prisma.emailAccount.update to also persist
result.subscriptionId (e.g. set data: { watchEmailsExpirationDate:
result.expirationDate, ...(result.subscriptionId ? { watchEmailsSubscriptionId:
result.subscriptionId } : {}) }), and return the subscriptionId in the response
so callers and unwatch logic can rely on it.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/app/api/outlook/webhook/route.ts (2)
14-19: Validation handshake: avoid double-decoding validationToken.
URL.searchParams.get()already decodes; applyingdecodeURIComponentagain can corrupt the token and break Graph’s validation challenge.Apply this diff:
if (validationToken) { logger.info("Received validation request", { validationToken }); - return new NextResponse(decodeURIComponent(validationToken), { + return new NextResponse(validationToken, { headers: { "Content-Type": "text/plain" }, }); }
25-36: Don’t return Zod error details in the HTTP response.Per API error-handling guidelines, avoid leaking internals. Keep detailed errors in logs; return a generic message.
Apply this diff:
- return NextResponse.json( - { - error: "Invalid webhook payload", - details: parseResult.error.errors, - }, - { status: 400 }, - ); + return NextResponse.json({ error: "Invalid webhook payload" }, { status: 400 });
♻️ Duplicate comments (1)
apps/web/app/api/outlook/webhook/types.ts (1)
23-24: Zod nullish semantics fix LGTM (resolves Graph omission cases).Switching to
.nullish()correctly accepts missing or null values in Graph payloads and aligns with prior feedback.Also applies to: 31-35
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/app/api/outlook/webhook/route.ts(1 hunks)apps/web/app/api/outlook/webhook/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/app/api/outlook/webhook/route.tsapps/web/app/api/outlook/webhook/types.ts
apps/web/app/**
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
NextJS app router structure with (app) directory
Files:
apps/web/app/api/outlook/webhook/route.tsapps/web/app/api/outlook/webhook/types.ts
apps/web/app/api/**/route.ts
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/app/api/**/route.ts: UsewithAuthfor user-level operations
UsewithEmailAccountfor email-account-level operations
Do NOT use POST API routes for mutations - use server actions instead
No need for try/catch in GET routes when using middleware
Export response types from GET routes
apps/web/app/api/**/route.ts: Wrap all GET API route handlers withwithAuthorwithEmailAccountmiddleware for authentication and authorization.
Export response types from GET API routes for type-safe client usage.
Do not use try/catch in GET API routes when using authentication middleware; rely on centralized error handling.
Files:
apps/web/app/api/outlook/webhook/route.ts
!{.cursor/rules/*.mdc}
📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/app/api/outlook/webhook/route.tsapps/web/app/api/outlook/webhook/types.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod
Files:
apps/web/app/api/outlook/webhook/route.tsapps/web/app/api/outlook/webhook/types.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/app/api/outlook/webhook/route.tsapps/web/app/api/outlook/webhook/types.ts
**/api/**/route.ts
📄 CodeRabbit inference engine (.cursor/rules/security.mdc)
**/api/**/route.ts: ALL API routes that handle user data MUST use appropriate authentication and authorization middleware (withAuth or withEmailAccount).
ALL database queries in API routes MUST be scoped to the authenticated user/account (e.g., include userId or emailAccountId in query filters).
Always validate that resources belong to the authenticated user before performing operations (resource ownership validation).
UsewithEmailAccountmiddleware for API routes that operate on a specific email account (i.e., use or requireemailAccountId).
UsewithAuthmiddleware for API routes that operate at the user level (i.e., use or require onlyuserId).
UsewithErrormiddleware (with proper validation) for public endpoints, custom authentication, or cron endpoints.
Cron endpoints MUST usewithErrormiddleware and validate the cron secret usinghasCronSecret(request)orhasPostCronSecret(request).
Cron endpoints MUST capture unauthorized attempts withcaptureExceptionand return a 401 status for unauthorized requests.
All parameters in API routes MUST be validated for type, format, and length before use.
Request bodies in API routes MUST be validated using Zod schemas before use.
All Prisma queries in API routes MUST only return necessary fields and never expose sensitive data.
Error messages in API routes MUST not leak internal information or sensitive data; use generic error messages and SafeError where appropriate.
API routes MUST use a consistent error response format, returning JSON with an error message and status code.
AllfindUniqueandfindFirstPrisma calls in API routes MUST include ownership filters (e.g., userId or emailAccountId).
AllfindManyPrisma calls in API routes MUST be scoped to the authenticated user's data.
Never use direct object references in API routes without ownership checks (prevent IDOR vulnerabilities).
Prevent mass assignment vulnerabilities by only allowing explicitly whitelisted fields in update operations in AP...
Files:
apps/web/app/api/outlook/webhook/route.ts
apps/web/app/api/**/*.{ts,js}
📄 CodeRabbit inference engine (.cursor/rules/security-audit.mdc)
apps/web/app/api/**/*.{ts,js}: All API route handlers in 'apps/web/app/api/' must use authentication middleware: withAuth, withEmailAccount, or withError (with custom authentication logic).
All Prisma queries in API routes must include user/account filtering (e.g., emailAccountId or userId in WHERE clauses) to prevent unauthorized data access.
All parameters used in API routes must be validated before use; do not use parameters from 'params' or request bodies directly in queries without validation.
Request bodies in API routes should use Zod schemas for validation.
API routes should only return necessary fields using Prisma's 'select' and must not include sensitive data in error messages.
Error messages in API routes must not reveal internal details; use generic errors and SafeError for user-facing errors.
All QStash endpoints (API routes called via publishToQstash or publishToQstashQueue) must use verifySignatureAppRouter to verify request authenticity.
All cron endpoints in API routes must use hasCronSecret or hasPostCronSecret for authentication.
Do not hardcode weak or plaintext secrets in API route files; secrets must not be directly assigned as string literals.
Review all new withError usage in API routes to ensure custom authentication is implemented where required.
Files:
apps/web/app/api/outlook/webhook/route.tsapps/web/app/api/outlook/webhook/types.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/app/api/outlook/webhook/route.tsapps/web/app/api/outlook/webhook/types.ts
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/app/api/outlook/webhook/route.tsapps/web/app/api/outlook/webhook/types.ts
🧬 Code graph analysis (2)
apps/web/app/api/outlook/webhook/route.ts (2)
apps/web/env.ts (1)
env(16-227)apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)
apps/web/app/api/outlook/webhook/types.ts (2)
apps/web/utils/types.ts (1)
RuleWithActionsAndCategories(16-18)apps/web/utils/llms/types.ts (1)
EmailAccountWithAI(10-29)
🔇 Additional comments (3)
apps/web/app/api/outlook/webhook/types.ts (2)
7-18: ProcessHistoryOptions shape looks good.Solid use of import type, minimal surface, and intersection to enrich emailAccount fields without duplicating Prisma selections.
42-42: Schema-derived OutlookResourceData improves type-source parity.Using
z.inferfromresourceDataSchemaprevents drift between validation and types.apps/web/app/api/outlook/webhook/route.ts (1)
60-61: Structured per-notification logging — LGTM.Helpful metadata for triage without leaking payload contents.
| for (const notification of body.value) { | ||
| if ( | ||
| !notification.clientState || | ||
| notification.clientState !== env.MICROSOFT_WEBHOOK_CLIENT_STATE | ||
| ) { | ||
| logger.error("Invalid or missing clientState", { | ||
| receivedClientState: notification.clientState, | ||
| hasExpectedClientState: !!env.MICROSOFT_WEBHOOK_CLIENT_STATE, | ||
| subscriptionId: notification.subscriptionId, | ||
| }); | ||
| return NextResponse.json( | ||
| { error: "Unauthorized webhook request" }, | ||
| { status: 403 }, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard for missing MICROSOFT_WEBHOOK_CLIENT_STATE to avoid false 403s.
If the env var isn’t set, every notification fails the check; treat as server misconfig and fail fast.
Apply this diff:
- // Validate clientState for security (verify webhook is from Microsoft)
- for (const notification of body.value) {
- if (
- !notification.clientState ||
- notification.clientState !== env.MICROSOFT_WEBHOOK_CLIENT_STATE
- ) {
+ // Validate clientState for security (verify webhook is from Microsoft)
+ const expectedClientState = env.MICROSOFT_WEBHOOK_CLIENT_STATE;
+ if (!expectedClientState) {
+ logger.error("Missing MICROSOFT_WEBHOOK_CLIENT_STATE; refusing to process notifications");
+ return NextResponse.json({ error: "Server misconfiguration" }, { status: 500 });
+ }
+ for (const notification of body.value) {
+ if (notification.clientState !== expectedClientState) {
logger.error("Invalid or missing clientState", {
receivedClientState: notification.clientState,
- hasExpectedClientState: !!env.MICROSOFT_WEBHOOK_CLIENT_STATE,
+ hasExpectedClientState: !!expectedClientState,
subscriptionId: notification.subscriptionId,
});
return NextResponse.json(
{ error: "Unauthorized webhook request" },
{ status: 403 },
);
}
}📝 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.
| for (const notification of body.value) { | |
| if ( | |
| !notification.clientState || | |
| notification.clientState !== env.MICROSOFT_WEBHOOK_CLIENT_STATE | |
| ) { | |
| logger.error("Invalid or missing clientState", { | |
| receivedClientState: notification.clientState, | |
| hasExpectedClientState: !!env.MICROSOFT_WEBHOOK_CLIENT_STATE, | |
| subscriptionId: notification.subscriptionId, | |
| }); | |
| return NextResponse.json( | |
| { error: "Unauthorized webhook request" }, | |
| { status: 403 }, | |
| ); | |
| } | |
| } | |
| // Validate clientState for security (verify webhook is from Microsoft) | |
| const expectedClientState = env.MICROSOFT_WEBHOOK_CLIENT_STATE; | |
| if (!expectedClientState) { | |
| logger.error("Missing MICROSOFT_WEBHOOK_CLIENT_STATE; refusing to process notifications"); | |
| return NextResponse.json( | |
| { error: "Server misconfiguration" }, | |
| { status: 500 }, | |
| ); | |
| } | |
| for (const notification of body.value) { | |
| if (notification.clientState !== expectedClientState) { | |
| logger.error("Invalid or missing clientState", { | |
| receivedClientState: notification.clientState, | |
| hasExpectedClientState: !!expectedClientState, | |
| subscriptionId: notification.subscriptionId, | |
| }); | |
| return NextResponse.json( | |
| { error: "Unauthorized webhook request" }, | |
| { status: 403 }, | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/api/outlook/webhook/route.ts around lines 42 to 57, the code
checks notification.clientState against env.MICROSOFT_WEBHOOK_CLIENT_STATE but
doesn't guard for the env var being unset, causing all requests to be rejected;
add an explicit check before iterating: if env.MICROSOFT_WEBHOOK_CLIENT_STATE is
falsy, log an error about missing configuration (include guidance to set the env
var) and return a 500 response (server misconfiguration) so webhook callers
aren’t incorrectly treated as unauthorized; otherwise proceed with the existing
clientState validation logic.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/web/app/api/outlook/webhook/process-history.ts (1)
131-139: Remove unsupported accessToken param and reuse the already-created providerProcessHistoryOptions doesn’t include accessToken; this will fail type-checking. Also, reuse the provider created earlier.
- try { - const emailProvider = await createEmailProvider({ - emailAccountId: emailAccount.id, - provider: emailAccount.account?.provider || "microsoft", - }); - - await processHistoryItem(resourceData, { - provider: emailProvider, - accessToken: emailAccount.account.access_token, + try { + await processHistoryItem(resourceData, { + provider, hasAutomationRules, hasAiAccess: userHasAiAccess, rules: emailAccount.rules, emailAccount, });apps/web/utils/ai/choose-rule/run-rules.ts (2)
70-79: Complete the provider refactor (rename client→provider end-to-end)Mixed naming breaks types where downstream helpers expect provider. Align param names and object keys.
if (result.rule) { return await executeMatchedRule( result.rule, message, emailAccount, - provider, + provider, result.reason, result.matchReasons, isTest, modelType, ); } @@ -async function executeMatchedRule( +async function executeMatchedRule( rule: RuleWithActionsAndCategories, message: ParsedMessage, emailAccount: EmailAccountWithAI, - client: EmailProvider, + provider: EmailProvider, reason: string | undefined, matchReasons: MatchReason[] | undefined, isTest: boolean, modelType: ModelType, ) { @@ const actionItems = await getActionItemsWithAiArgs({ message, emailAccount, selectedRule: rule, - client, + provider, modelType, }); @@ if (shouldExecute) { await executeAct({ - client, + provider, userEmail: emailAccount.email, userId: emailAccount.userId, emailAccountId: emailAccount.id, executedRule, message, }); }Also applies to: 91-100, 102-109, 156-163
1-334: Renameclienttoproviderin remaining helpers
getActionItemsWithAiArgs (apps/web/utils/ai/choose-rule/choose-args.ts) and executeAct (apps/web/utils/ai/choose-rule/execute.ts) still accept aclient: EmailProvider. Update both signatures and all call sites to useprovider: EmailProviderfor consistency with findMatchingRule.apps/web/utils/ai/choose-rule/match-rules.ts (3)
40-50: Make provider optional in findPotentialMatchingRules; forward safelyThis function only needs provider for reply-preset filtering. Accept undefined to keep non-provider call sites working.
-async function findPotentialMatchingRules({ +async function findPotentialMatchingRules({ rules, message, isThread, - provider, + provider, }: { rules: RuleWithActionsAndCategories[]; message: ParsedMessage; isThread: boolean; - provider: EmailProvider; + provider?: EmailProvider; }): Promise<MatchingRuleResult> { @@ - const filteredPotentialMatches = await filterToReplyPreset( + const filteredPotentialMatches = await filterToReplyPreset( potentialMatches, message, - provider, + provider, );Also applies to: 174-181
202-214: Propagate optional provider through public findMatchingRule APIKeep the public API tolerant to older callers (tests) that don’t pass provider yet.
export async function findMatchingRule({ rules, message, emailAccount, - provider, + provider, modelType, }: { rules: RuleWithActionsAndCategories[]; message: ParsedMessage; emailAccount: EmailAccountWithAI; - provider: EmailProvider; + provider?: EmailProvider; modelType: ModelType; }) {
228-234: Guard provider usage and fix missing provider in run-rules.ts
In
apps/web/utils/ai/choose-rule/match-rules.ts, relax both function signatures to acceptprovider?: EmailProviderand replaceconst isThread = provider.isReplyInThread(message);with
const isThread = provider?.isReplyInThread?.(message) ?? false;(apply to
findMatchingRuleWithReasonsand the exportedfindMatchingRuledestructured param).In
apps/web/utils/ai/choose-rule/run-rules.ts(around line 52), pass the provider intofindMatchingRule:await findMatchingRule({ rules, message,
provider: emailAccount.provider, emailAccount, modelType,
♻️ Duplicate comments (1)
apps/web/app/api/outlook/webhook/process-history-item.ts (1)
89-101: Inbox/Sent detection is Gmail-specific; resolve Outlook folder by ID before skippingChecking labelIds for "INBOX"/"SENT" will miss Outlook, causing valid messages to be skipped. Resolve the folder via resourceData.folderId with provider.getLabelById(), then fall back to labelIds. This mirrors the earlier parentFolderId issue.
- // Skip messages that are not in inbox or sent items folders - // We want to process inbox messages (for rules/automation) and sent messages (for reply tracking) - // Note: ParsedMessage already contains the necessary folder information in labels - const isInInbox = parsedMessage.labelIds?.includes("INBOX") || false; - const isInSentItems = parsedMessage.labelIds?.includes("SENT") || false; - - if (!isInInbox && !isInSentItems) { - logger.info("Skipping message not in inbox or sent items", { - ...loggerOptions, - labels: parsedMessage.labelIds, - }); - return; - } + // Process only Inbox or Sent Items. + // Try Graph folderId first (Outlook), then fall back to Gmail-style labels. + let isInInbox = parsedMessage.labelIds?.includes("INBOX") || false; + let isInSentItems = parsedMessage.labelIds?.includes("SENT") || false; + + if (!isInInbox && !isInSentItems && resourceData.folderId) { + try { + const folder = await provider.getLabelById(resourceData.folderId); + const name = folder?.name?.toLowerCase(); + isInInbox = name === "inbox"; + isInSentItems = name === "sent items" || name === "sentitems" || name === "sent"; + } catch (e) { + logger.info("Could not resolve folder by id; falling back to labels", { + ...loggerOptions, + folderId: resourceData.folderId, + }); + } + } + + if (!isInInbox && !isInSentItems) { + logger.info("Skipping message not in inbox or sent items", { + ...loggerOptions, + labels: parsedMessage.labelIds, + folderId: resourceData.folderId, + }); + return; + }
🧹 Nitpick comments (8)
apps/web/app/api/outlook/webhook/block-unsubscribed-emails.ts (1)
23-29: Make email match case-insensitive to avoid missing unsubscribesEmail casing differences will bypass the unsubscribe check.
const sender = await prisma.newsletter.findFirst({ where: { emailAccountId, - email, + email: { equals: email, mode: "insensitive" }, status: NewsletterStatus.UNSUBSCRIBED, }, });apps/web/app/api/outlook/webhook/process-history.ts (3)
125-129: Drop duplicate provider creationProvider is already initialized at Lines 71-75; creating it again is redundant.
- const emailProvider = await createEmailProvider({ - emailAccountId: emailAccount.id, - provider: emailAccount.account?.provider || "microsoft", - });
2-2: Remove unused importgetOutlookClientWithRefresh is not used.
-import { getOutlookClientWithRefresh } from "@/utils/outlook/client";
71-75: Consider wrapping provider creation in try/catchIf provider instantiation throws (e.g., unsupported provider), you’ll fail before cleanup (unwatch). Wrap and degrade gracefully.
apps/web/utils/ai/choose-rule/run-rules.ts (1)
110-112: Remove empty blockDead code; no-op.
- if (!isTest) { - }apps/web/app/api/outlook/webhook/process-history-item.ts (3)
145-154: More robust outbound detection (fallback to sender equals account email)If Sent classification fails, use sender==userEmail as a safe fallback to ensure reply-tracking runs.
- const isOutbound = isInSentItems; + const isOutbound = + isInSentItems || + extractEmailAddress(from).toLowerCase() === userEmail.toLowerCase();
180-187: Avoid redundant date overridegetEmailForLLM already maps internalDate->Date; you’re reassigning date immediately after. Minor but simplifies state.
- const emailForLLM = getEmailForLLM(parsedMessage); + const emailForLLM = getEmailForLLM(parsedMessage); @@ - email: { - ...emailForLLM, - threadId: resourceData.conversationId || messageId, - date: parsedMessage.date ? new Date(parsedMessage.date) : new Date(), - }, + email: { ...emailForLLM, threadId: resourceData.conversationId || messageId },
217-241: Simplify constructed message: dedupe date/snippet fallbackMinor cleanup: you set headers.date and top-level date twice. Consider one canonical assignment via parsedMessage.
Do you want me to push a small refactor PR to reduce duplication here?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
apps/web/app/api/outlook/webhook/block-unsubscribed-emails.ts(2 hunks)apps/web/app/api/outlook/webhook/process-history-item.ts(10 hunks)apps/web/app/api/outlook/webhook/process-history.ts(3 hunks)apps/web/app/api/outlook/webhook/types.ts(1 hunks)apps/web/utils/ai/choose-rule/match-rules.ts(6 hunks)apps/web/utils/ai/choose-rule/run-rules.ts(3 hunks)apps/web/utils/outlook/message.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/utils/outlook/message.ts
🧰 Additional context used
📓 Path-based instructions (11)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/app/api/outlook/webhook/block-unsubscribed-emails.tsapps/web/utils/ai/choose-rule/match-rules.tsapps/web/app/api/outlook/webhook/types.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/process-history.ts
apps/web/app/**
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
NextJS app router structure with (app) directory
Files:
apps/web/app/api/outlook/webhook/block-unsubscribed-emails.tsapps/web/app/api/outlook/webhook/types.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/process-history.ts
!{.cursor/rules/*.mdc}
📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/app/api/outlook/webhook/block-unsubscribed-emails.tsapps/web/utils/ai/choose-rule/match-rules.tsapps/web/app/api/outlook/webhook/types.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/process-history.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod
Files:
apps/web/app/api/outlook/webhook/block-unsubscribed-emails.tsapps/web/utils/ai/choose-rule/match-rules.tsapps/web/app/api/outlook/webhook/types.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/process-history.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/app/api/outlook/webhook/block-unsubscribed-emails.tsapps/web/utils/ai/choose-rule/match-rules.tsapps/web/app/api/outlook/webhook/types.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/process-history.ts
apps/web/app/api/**/*.{ts,js}
📄 CodeRabbit inference engine (.cursor/rules/security-audit.mdc)
apps/web/app/api/**/*.{ts,js}: All API route handlers in 'apps/web/app/api/' must use authentication middleware: withAuth, withEmailAccount, or withError (with custom authentication logic).
All Prisma queries in API routes must include user/account filtering (e.g., emailAccountId or userId in WHERE clauses) to prevent unauthorized data access.
All parameters used in API routes must be validated before use; do not use parameters from 'params' or request bodies directly in queries without validation.
Request bodies in API routes should use Zod schemas for validation.
API routes should only return necessary fields using Prisma's 'select' and must not include sensitive data in error messages.
Error messages in API routes must not reveal internal details; use generic errors and SafeError for user-facing errors.
All QStash endpoints (API routes called via publishToQstash or publishToQstashQueue) must use verifySignatureAppRouter to verify request authenticity.
All cron endpoints in API routes must use hasCronSecret or hasPostCronSecret for authentication.
Do not hardcode weak or plaintext secrets in API route files; secrets must not be directly assigned as string literals.
Review all new withError usage in API routes to ensure custom authentication is implemented where required.
Files:
apps/web/app/api/outlook/webhook/block-unsubscribed-emails.tsapps/web/app/api/outlook/webhook/types.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/process-history.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/app/api/outlook/webhook/block-unsubscribed-emails.tsapps/web/utils/ai/choose-rule/match-rules.tsapps/web/app/api/outlook/webhook/types.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/process-history.ts
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/app/api/outlook/webhook/block-unsubscribed-emails.tsapps/web/utils/ai/choose-rule/match-rules.tsapps/web/app/api/outlook/webhook/types.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/app/api/outlook/webhook/process-history-item.tsapps/web/app/api/outlook/webhook/process-history.ts
apps/web/utils/**
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Create utility functions in
utils/folder for reusable logic
Files:
apps/web/utils/ai/choose-rule/match-rules.tsapps/web/utils/ai/choose-rule/run-rules.ts
apps/web/utils/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size
Files:
apps/web/utils/ai/choose-rule/match-rules.tsapps/web/utils/ai/choose-rule/run-rules.ts
apps/web/utils/{ai,llms}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm.mdc)
apps/web/utils/{ai,llms}/**/*.ts: Place LLM-related implementation code under apps/web/utils/ai or apps/web/utils/llms
Keep system and user prompts separate; system defines role/task, user contains data/context
Always validate LLM responses with a specific Zod schema
Use descriptive scoped loggers per feature and log inputs/outputs with appropriate levels and context
Implement early returns for invalid inputs and use proper error types with logging
Add fallbacks for AI failures and include retry logic for transient errors using withRetry
Format prompts with XML-like tags; remove excessive whitespace; truncate overly long inputs; keep formatting consistent
Use TypeScript types for all parameters/returns and define interfaces for complex IO structures
Keep related AI functions co-located; extract shared logic into utilities; document complex AI logic with clear comments
Call LLMs via createGenerateObject; pass system, prompt, and a Zod schema; return the validated result.object
Derive model options using getModel(...) and pass them to createGenerateObject and the generate call
Files:
apps/web/utils/ai/choose-rule/match-rules.tsapps/web/utils/ai/choose-rule/run-rules.ts
🧬 Code graph analysis (5)
apps/web/utils/ai/choose-rule/match-rules.ts (5)
apps/web/utils/types.ts (2)
RuleWithActionsAndCategories(16-18)ParsedMessage(55-70)apps/web/utils/email/types.ts (1)
EmailProvider(36-171)apps/web/utils/llms/types.ts (1)
EmailAccountWithAI(10-29)apps/web/utils/llms/model.ts (1)
ModelType(17-17)apps/web/utils/ai/choose-rule/types.ts (1)
MatchReason(29-34)
apps/web/app/api/outlook/webhook/types.ts (2)
apps/web/utils/types.ts (1)
RuleWithActionsAndCategories(16-18)apps/web/utils/llms/types.ts (1)
EmailAccountWithAI(10-29)
apps/web/utils/ai/choose-rule/run-rules.ts (1)
apps/web/utils/email/types.ts (1)
EmailProvider(36-171)
apps/web/app/api/outlook/webhook/process-history-item.ts (6)
apps/web/utils/get-email-from-message.ts (1)
getEmailForLLM(6-25)apps/web/utils/cold-email/is-cold-email.ts (1)
runColdEmailBlocker(161-187)apps/web/utils/categorize/senders/categorize.ts (1)
categorizeSender(17-57)apps/web/utils/ai/choose-rule/run-rules.ts (1)
runRules(37-89)apps/web/utils/types.ts (1)
ParsedMessage(55-70)apps/web/utils/email/types.ts (1)
EmailProvider(36-171)
apps/web/app/api/outlook/webhook/process-history.ts (2)
apps/web/utils/email/provider.ts (1)
createEmailProvider(13-28)apps/web/app/api/outlook/webhook/process-history-item.ts (1)
processHistoryItem(26-256)
🪛 GitHub Check: test
apps/web/utils/ai/choose-rule/match-rules.ts
[failure] 239-239: utils/ai/choose-rule/match-rules.test.ts > findMatchingRule > should match with only one of category or group
TypeError: Cannot read properties of undefined (reading 'isReplyInThread')
❯ findMatchingRuleWithReasons utils/ai/choose-rule/match-rules.ts:239:29
❯ Module.findMatchingRule utils/ai/choose-rule/match-rules.ts:215:24
❯ utils/ai/choose-rule/match-rules.test.ts:768:26
[failure] 239-239: utils/ai/choose-rule/match-rules.test.ts > findMatchingRule > doesn't match when AI condition fails (category matches but AI doesn't)
TypeError: Cannot read properties of undefined (reading 'isReplyInThread')
❯ findMatchingRuleWithReasons utils/ai/choose-rule/match-rules.ts:239:29
❯ Module.findMatchingRule utils/ai/choose-rule/match-rules.ts:215:24
❯ utils/ai/choose-rule/match-rules.test.ts:741:26
[failure] 239-239: utils/ai/choose-rule/match-rules.test.ts > findMatchingRule > matches a rule with multiple conditions AND (category and AI)
TypeError: Cannot read properties of undefined (reading 'isReplyInThread')
❯ findMatchingRuleWithReasons utils/ai/choose-rule/match-rules.ts:239:29
❯ Module.findMatchingRule utils/ai/choose-rule/match-rules.ts:215:24
❯ utils/ai/choose-rule/match-rules.test.ts:705:26
[failure] 239-239: utils/ai/choose-rule/match-rules.test.ts > findMatchingRule > matches a rule with multiple conditions AND (category and group)
TypeError: Cannot read properties of undefined (reading 'isReplyInThread')
❯ findMatchingRuleWithReasons utils/ai/choose-rule/match-rules.ts:239:29
❯ Module.findMatchingRule utils/ai/choose-rule/match-rules.ts:215:24
❯ utils/ai/choose-rule/match-rules.test.ts:668:26
[failure] 239-239: utils/ai/choose-rule/match-rules.test.ts > findMatchingRule > matches a smart category rule with exclude
TypeError: Cannot read properties of undefined (reading 'isReplyInThread')
❯ findMatchingRuleWithReasons utils/ai/choose-rule/match-rules.ts:239:29
❯ Module.findMatchingRule utils/ai/choose-rule/match-rules.ts:215:24
❯ utils/ai/choose-rule/match-rules.test.ts:627:26
[failure] 239-239: utils/ai/choose-rule/match-rules.test.ts > findMatchingRule > matches a smart category rule
TypeError: Cannot read properties of undefined (reading 'isReplyInThread')
❯ findMatchingRuleWithReasons utils/ai/choose-rule/match-rules.ts:239:29
❯ Module.findMatchingRule utils/ai/choose-rule/match-rules.ts:215:24
❯ utils/ai/choose-rule/match-rules.test.ts:603:26
[failure] 239-239: utils/ai/choose-rule/match-rules.test.ts > findMatchingRule > matches a group rule
TypeError: Cannot read properties of undefined (reading 'isReplyInThread')
❯ findMatchingRuleWithReasons utils/ai/choose-rule/match-rules.ts:239:29
❯ Module.findMatchingRule utils/ai/choose-rule/match-rules.ts:215:24
❯ utils/ai/choose-rule/match-rules.test.ts:577:26
[failure] 239-239: utils/ai/choose-rule/match-rules.test.ts > findMatchingRule > doens't match wrong static domain
TypeError: Cannot read properties of undefined (reading 'isReplyInThread')
❯ findMatchingRuleWithReasons utils/ai/choose-rule/match-rules.ts:239:29
❯ Module.findMatchingRule utils/ai/choose-rule/match-rules.ts:215:24
❯ utils/ai/choose-rule/match-rules.test.ts:547:26
[failure] 239-239: utils/ai/choose-rule/match-rules.test.ts > findMatchingRule > matches a static domain
TypeError: Cannot read properties of undefined (reading 'isReplyInThread')
❯ findMatchingRuleWithReasons utils/ai/choose-rule/match-rules.ts:239:29
❯ Module.findMatchingRule utils/ai/choose-rule/match-rules.ts:215:24
❯ utils/ai/choose-rule/match-rules.test.ts:528:26
[failure] 239-239: utils/ai/choose-rule/match-rules.test.ts > findMatchingRule > matches a static rule
TypeError: Cannot read properties of undefined (reading 'isReplyInThread')
❯ findMatchingRuleWithReasons utils/ai/choose-rule/match-rules.ts:239:29
❯ Module.findMatchingRule utils/ai/choose-rule/match-rules.ts:215:24
❯ utils/ai/choose-rule/match-rules.test.ts:509:26
🪛 GitHub Actions: Run Tests
apps/web/utils/ai/choose-rule/match-rules.ts
[error] 239-239: Cannot read properties of undefined (reading 'isReplyInThread')
🔇 Additional comments (3)
apps/web/app/api/outlook/webhook/process-history-item.ts (1)
271-310: Good: outbound handling is concurrent and errors are isolatedParallelizing draft tracking and reply handling with Promise.allSettled and logging failures is solid defensive ops.
apps/web/app/api/outlook/webhook/types.ts (2)
7-18: ProcessHistoryOptions looks good and aligns with provider-first flowType shape matches downstream usage; narrowing EmailAccount fields is appropriate.
20-26: Approve code changes as-is
The grep output showsresourceDatais assumed present across webhook handling and downstream processing (e.g. destructuring inroute.ts, calls toprocessHistoryForUserandprocessHistoryItemwithout null checks). Making it optional without adjusting all usages risks runtime errors. KeepresourceDatarequired.
| export async function filterToReplyPreset( | ||
| potentialMatches: (RuleWithActionsAndCategories & { instructions: string })[], | ||
| message: ParsedMessage, | ||
| client: EmailProvider, | ||
| provider: EmailProvider, | ||
| ): Promise<(RuleWithActionsAndCategories & { instructions: string })[]> { | ||
| const toReplyRule = potentialMatches.find( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard reply-history check when provider is absent; avoid throwing and keep behavior
If provider is undefined (e.g., unit tests), skip the TO_REPLY suppression logic rather than error.
-export async function filterToReplyPreset(
+export async function filterToReplyPreset(
potentialMatches: (RuleWithActionsAndCategories & { instructions: string })[],
message: ParsedMessage,
- provider: EmailProvider,
+ provider?: EmailProvider,
): Promise<(RuleWithActionsAndCategories & { instructions: string })[]> {
@@
- try {
+ try {
+ if (!provider) {
+ // No provider available (e.g., tests); keep potential matches unchanged.
+ return potentialMatches;
+ }
const { hasReplied, receivedCount } = await checkSenderReplyHistory(
provider,
senderEmail,
TO_REPLY_RECEIVED_THRESHOLD,
);Also applies to: 399-405, 417-425
🤖 Prompt for AI Agents
In apps/web/utils/ai/choose-rule/match-rules.ts around lines 366-371 (and
similarly at 399-405 and 417-425), the code assumes EmailProvider is defined and
accesses provider.replyHistory which will throw when provider is undefined;
guard these checks by first verifying provider is truthy and only then applying
the TO_REPLY suppression logic (e.g., if (!provider) return potentialMatches or
skip the reply-history branch; otherwise continue with the existing filtering).
Ensure all three ranges perform a provider presence check before referencing
provider.replyHistory so unit tests and other callers without a provider do not
error and current behavior is preserved when provider exists.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/utils/outlook/thread.ts (1)
16-21: Make sort resilient to invalid dates and avoid mutating the SDK arrayGuard against NaN when parsing dates and avoid in-place mutation of
messages.value.- // Sort in memory to avoid "restriction or sort order is too complex" error - return messages.value.sort((a, b) => { - const dateA = new Date(a.receivedDateTime || 0).getTime(); - const dateB = new Date(b.receivedDateTime || 0).getTime(); - return dateB - dateA; // desc order (newest first) - }); + // Sort in memory to avoid "restriction or sort order is too complex" error + // Use a copy and guard against invalid/empty dates. + return [...messages.value].sort((a, b) => { + const dateA = Date.parse(a.receivedDateTime || "") || 0; + const dateB = Date.parse(b.receivedDateTime || "") || 0; + return dateB - dateA; // desc order (newest first) + });apps/web/utils/email/microsoft.ts (2)
372-377: Sort defensively and avoid mutating the local arrayHandle invalid
datevalues and return a sorted copy to keep behavior predictable.- // Sort messages by receivedDateTime in ascending order (oldest first) to avoid "restriction or sort order is too complex" error - return messages.sort((a, b) => { - const dateA = new Date(a.date || 0).getTime(); - const dateB = new Date(b.date || 0).getTime(); - return dateA - dateB; // asc order (oldest first) - }); + // Sort messages by date ascending (oldest first); guard against invalid dates + return [...messages].sort((a, b) => { + const ta = Date.parse(a.date || "") || 0; + const tb = Date.parse(b.date || "") || 0; + return ta - tb; + });
901-901: Confirm nullability contract for resourceData.conversationIdYou set
conversationIdtonullwhen absent. Ensure the downstream schema/type acceptsstring | nullto avoid subtle runtime/validation issues. If null isn’t required, prefer omitting the property.Option (omit when absent):
- conversationId: options.startHistoryId?.toString() || null, + ...(options.startHistoryId + ? { conversationId: options.startHistoryId.toString() } + : {}),If null is intended, consider updating this method’s
options.resourceDatatype toconversationId?: string | nullfor consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/web/app/api/outlook/webhook/process-history-item.ts(10 hunks)apps/web/utils/email/microsoft.ts(3 hunks)apps/web/utils/outlook/message.ts(2 hunks)apps/web/utils/outlook/thread.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/utils/outlook/message.ts
- apps/web/app/api/outlook/webhook/process-history-item.ts
🧰 Additional context used
📓 Path-based instructions (8)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/utils/outlook/thread.tsapps/web/utils/email/microsoft.ts
!{.cursor/rules/*.mdc}
📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/utils/outlook/thread.tsapps/web/utils/email/microsoft.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod
Files:
apps/web/utils/outlook/thread.tsapps/web/utils/email/microsoft.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/utils/outlook/thread.tsapps/web/utils/email/microsoft.ts
apps/web/utils/**
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Create utility functions in
utils/folder for reusable logic
Files:
apps/web/utils/outlook/thread.tsapps/web/utils/email/microsoft.ts
apps/web/utils/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size
Files:
apps/web/utils/outlook/thread.tsapps/web/utils/email/microsoft.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/utils/outlook/thread.tsapps/web/utils/email/microsoft.ts
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/utils/outlook/thread.tsapps/web/utils/email/microsoft.ts
🔇 Additional comments (1)
apps/web/utils/email/microsoft.ts (1)
59-59: Import reorder is fineNo functional impact.
|
@claude review |
|
Claude finished @elie222's task —— View job Code Review for PR #739: Sent events outlookTodo List:
OverviewThis is a substantial and well-architected refactor that centralizes Outlook subscription management and introduces a clean provider abstraction. The changes improve maintainability, reduce code duplication, and provide better error handling. However, there are several critical issues that need to be addressed before merge. 🚨 Critical Issues1. Folder Detection Logic Broken (
|
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores