Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughRefactors Gmail forwarding to pass a full ParsedMessage into forwardEmail; threads a typed Logger through Google webhook/history and many request handlers/utilities (replacing module-level loggers with per-call scoped loggers); adds token-cleanup helper and bumps version to v2.16.7. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant GoogleWebhook as "route.ts (webhook)"
participant HistoryProc as "process-history.ts"
participant ItemProc as "process-history-item.ts"
participant LabelHandler as "process-label-removed-event.ts"
User->>GoogleWebhook: POST webhook (decodedData)
GoogleWebhook->>GoogleWebhook: createScopedLogger(...).with({email,historyId})
GoogleWebhook->>HistoryProc: processHistoryForUser(decodedData, options, logger)
HistoryProc->>HistoryProc: logger.with({messageId,threadId})
HistoryProc->>ItemProc: processHistoryItem(historyItem, options, perEventLogger)
ItemProc->>LabelHandler: handleLabelRemovedEvent(..., perEventLogger)
LabelHandler->>LabelHandler: learnFromRemovedLabel(..., logger.with(...))
LabelHandler-->>ItemProc: result
ItemProc-->>HistoryProc: result
HistoryProc-->>GoogleWebhook: done
sequenceDiagram
autonumber
actor User
participant GoogleEmail as "apps/web/utils/email/google.ts"
participant GmailAPI as "Gmail API"
participant MailUtil as "apps/web/utils/gmail/mail.ts"
User->>GoogleEmail: request forward(email.id, recipient)
GoogleEmail->>GmailAPI: getMessage(email.id)
GmailAPI-->>GoogleEmail: ParsedMessage
Note over GoogleEmail,MailUtil: Full ParsedMessage is now passed (was messageId only)
GoogleEmail->>MailUtil: forwardEmail(recipient, message: ParsedMessage, options...)
MailUtil->>GmailAPI: fetch attachments using message.id
MailUtil->>GmailAPI: send forwarded email
GmailAPI-->>MailUtil: send result
MailUtil-->>GoogleEmail: forward result
GoogleEmail-->>User: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/utils/cold-email/is-cold-email.ts (1)
18-87: WrapisColdEmailin a try/catch.
The implementation inapps/web/utils/cold-email/is-cold-email.tscurrently invokes multiple async calls without error handling, and its callers don’t catch exceptions. Surround the function body with a try/catch block, log errors, and return a safe fallback.apps/web/app/api/google/webhook/process-label-removed-event.ts (1)
22-94: Mandatory logger breaks existing callersMaking
handleLabelRemovedEventrequire a logger causes runtime failures when legacy callers/tests still invoke it with two arguments—CI already reportsCannot read properties of undefined (reading 'info'). Please keep backwards compatibility by defaulting to a scoped logger (or update every caller, including tests, in this PR). A minimal fix looks like:-import type { Logger } from "@/utils/logger"; +import { createScopedLogger, type Logger } from "@/utils/logger"; … - logger: Logger, + logger?: Logger, ) { - const messageId = message.message?.id; - const threadId = message.message?.threadId; + const log = logger ?? createScopedLogger("google/webhook/label-removed"); + const messageId = message.message?.id; + const threadId = message.message?.threadId; … - logger.warn("Skipping label removal - missing messageId or threadId"); + log.warn("Skipping label removal - missing messageId or threadId"); … - logger.info("Processing label removal for learning"); + log.info("Processing label removal for learning"); … - logger.error("Error getting sender for label removal", { + log.error("Error getting sender for label removal", { … - logger.trace("No non-system labels to process"); + log.trace("No non-system labels to process"); … - await learnFromRemovedLabel({ + await learnFromRemovedLabel({ … - logger, + logger: log, }); … - logger.error("Error learning from label removal", { + log.error("Error learning from label removal", {apps/web/app/api/google/webhook/process-history-item.ts (1)
9-61: processHistoryItem now crashes when logger omittedThe new
logger: Loggerparameter isn’t supplied by older call sites/tests, sologger.infoblows up (Cannot read properties of undefined (reading 'info')in CI). Please keep the function tolerant by defaulting to a scoped logger (or update every caller within this PR). Suggested change:-import type { Logger } from "@/utils/logger"; +import { createScopedLogger, type Logger } from "@/utils/logger"; … - logger: Logger, + logger?: Logger, ) { + const log = logger ?? createScopedLogger("google/webhook/history-item"); … - logger.info("Processing label removed event for learning"); + log.info("Processing label removed event for learning"); return handleLabelRemovedEvent( item, { emailAccount, provider, }, - logger, + log, ); } else if (type === HistoryEventType.LABEL_ADDED) { - logger.info("Processing label added event for learning"); + log.info("Processing label added event for learning"); return; } return processHistoryItemShared( { messageId, threadId }, { provider, emailAccount, hasAutomationRules, hasAiAccess, rules, - logger, + logger: log, }, );
🧹 Nitpick comments (5)
apps/web/utils/assistant/process-assistant-email.ts (2)
61-61: Consider adding more context to this log statement.While the logger already includes
emailAccountId,threadId, andmessageIdvia.with(), consider adding any processing-specific details that might be useful for debugging, such as the number of messages in the thread or processing timestamp.
70-70: Consider enriching error context.The error message "No thread messages found" could benefit from additional context. Consider adding the
threadIdor message count to help with debugging.- logger.error("No thread messages found"); + logger.error("No thread messages found", { threadId: message.threadId });apps/web/utils/ai/assistant/chat.ts (1)
237-237: LGTM! Logging simplification improves readability.The error logging is properly structured and aligns with the PR's goal of improving error handling. The error is caught, logged with context, and returned to the caller.
Optional: Consider adding rule context to error logs for easier debugging.
While the current logging is sufficient, including the rule creation parameters could help with debugging:
- logger.error("Failed to create rule", { error: message }); + logger.error("Failed to create rule", { error: message, ruleName: name });Optional: Consider standardizing multi-line vs single-line format for consistency.
For consistency across the file, you could format line 221-223 to match the single-line style:
- logger.error("Error while creating rule", { - error: rule.error, - }); + logger.error("Error while creating rule", { error: rule.error });apps/web/utils/ai/choose-rule/ai-choose-args.ts (1)
82-82: Remove commented-out code.The commented-out logger.trace statement should either be uncommented if needed for debugging or removed entirely to keep the codebase clean.
Apply this diff to remove the commented-out code:
- // logger.trace("Parameters:", zodToJsonSchema(parameters));apps/web/utils/cold-email/is-cold-email.ts (1)
66-66: Consider adding context to log statements for better observability.Some log statements lack contextual information that could aid debugging:
- Line 66 doesn't log which sender had previous communication (unlike the log at line 50-52 which includes
from)- Lines 78-80 log
coldEmailbut omitres.reason, which contains the AI's explanationApply these changes for more consistent and informative logging:
- logger.info("Has previous email"); + logger.info("Has previous email", { from: email.from });- logger.info("AI is cold email?", { - coldEmail: res.coldEmail, - }); + logger.info("AI is cold email?", { + coldEmail: res.coldEmail, + reason: res.reason, + });Also applies to: 78-80
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/web/app/api/google/webhook/logger.ts(0 hunks)apps/web/app/api/google/webhook/process-history-item.ts(3 hunks)apps/web/app/api/google/webhook/process-history.ts(7 hunks)apps/web/app/api/google/webhook/process-label-removed-event.ts(5 hunks)apps/web/app/api/google/webhook/route.ts(2 hunks)apps/web/app/api/resend/digest/route.ts(10 hunks)apps/web/app/api/resend/summary/route.ts(5 hunks)apps/web/utils/actions/admin.ts(0 hunks)apps/web/utils/ai/assistant/chat.ts(1 hunks)apps/web/utils/ai/assistant/process-user-request.ts(6 hunks)apps/web/utils/ai/choose-rule/ai-choose-args.ts(2 hunks)apps/web/utils/ai/report/fetch.ts(1 hunks)apps/web/utils/assistant/process-assistant-email.ts(8 hunks)apps/web/utils/cold-email/is-cold-email.ts(2 hunks)apps/web/utils/email/google.ts(3 hunks)apps/web/utils/email/microsoft.ts(1 hunks)apps/web/utils/outlook/message.ts(1 hunks)apps/web/utils/reply-tracker/draft-tracking.ts(1 hunks)apps/web/utils/webhook/validate-webhook-account.ts(4 hunks)
💤 Files with no reviewable changes (2)
- apps/web/app/api/google/webhook/logger.ts
- apps/web/utils/actions/admin.ts
✅ Files skipped from review due to trivial changes (1)
- apps/web/utils/email/microsoft.ts
🧰 Additional context used
📓 Path-based instructions (14)
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/cold-email/is-cold-email.tsapps/web/utils/ai/assistant/chat.tsapps/web/utils/ai/report/fetch.tsapps/web/utils/outlook/message.tsapps/web/utils/assistant/process-assistant-email.tsapps/web/app/api/resend/digest/route.tsapps/web/utils/email/google.tsapps/web/utils/ai/choose-rule/ai-choose-args.tsapps/web/utils/ai/assistant/process-user-request.tsapps/web/app/api/google/webhook/process-history.tsapps/web/utils/reply-tracker/draft-tracking.tsapps/web/utils/webhook/validate-webhook-account.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/route.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/resend/summary/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/utils/cold-email/is-cold-email.tsapps/web/utils/ai/assistant/chat.tsapps/web/utils/ai/report/fetch.tsapps/web/utils/outlook/message.tsapps/web/utils/assistant/process-assistant-email.tsapps/web/app/api/resend/digest/route.tsapps/web/utils/email/google.tsapps/web/utils/ai/choose-rule/ai-choose-args.tsapps/web/utils/ai/assistant/process-user-request.tsapps/web/app/api/google/webhook/process-history.tsapps/web/utils/reply-tracker/draft-tracking.tsapps/web/utils/webhook/validate-webhook-account.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/route.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/resend/summary/route.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/cold-email/is-cold-email.tsapps/web/utils/ai/assistant/chat.tsapps/web/utils/ai/report/fetch.tsapps/web/utils/outlook/message.tsapps/web/utils/assistant/process-assistant-email.tsapps/web/app/api/resend/digest/route.tsapps/web/utils/email/google.tsapps/web/utils/ai/choose-rule/ai-choose-args.tsapps/web/utils/ai/assistant/process-user-request.tsapps/web/app/api/google/webhook/process-history.tsapps/web/utils/reply-tracker/draft-tracking.tsapps/web/utils/webhook/validate-webhook-account.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/route.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/resend/summary/route.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/cold-email/is-cold-email.tsapps/web/utils/ai/assistant/chat.tsapps/web/utils/ai/report/fetch.tsapps/web/utils/outlook/message.tsapps/web/utils/assistant/process-assistant-email.tsapps/web/app/api/resend/digest/route.tsapps/web/utils/email/google.tsapps/web/utils/ai/choose-rule/ai-choose-args.tsapps/web/utils/ai/assistant/process-user-request.tsapps/web/app/api/google/webhook/process-history.tsapps/web/utils/reply-tracker/draft-tracking.tsapps/web/utils/webhook/validate-webhook-account.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/route.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/resend/summary/route.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/cold-email/is-cold-email.tsapps/web/utils/ai/assistant/chat.tsapps/web/utils/ai/report/fetch.tsapps/web/utils/outlook/message.tsapps/web/utils/assistant/process-assistant-email.tsapps/web/utils/email/google.tsapps/web/utils/ai/choose-rule/ai-choose-args.tsapps/web/utils/ai/assistant/process-user-request.tsapps/web/utils/reply-tracker/draft-tracking.tsapps/web/utils/webhook/validate-webhook-account.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/cold-email/is-cold-email.tsapps/web/utils/ai/assistant/chat.tsapps/web/utils/ai/report/fetch.tsapps/web/utils/outlook/message.tsapps/web/utils/assistant/process-assistant-email.tsapps/web/utils/email/google.tsapps/web/utils/ai/choose-rule/ai-choose-args.tsapps/web/utils/ai/assistant/process-user-request.tsapps/web/utils/reply-tracker/draft-tracking.tsapps/web/utils/webhook/validate-webhook-account.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/cold-email/is-cold-email.tsapps/web/utils/ai/assistant/chat.tsapps/web/utils/ai/report/fetch.tsapps/web/utils/outlook/message.tsapps/web/utils/assistant/process-assistant-email.tsapps/web/app/api/resend/digest/route.tsapps/web/utils/email/google.tsapps/web/utils/ai/choose-rule/ai-choose-args.tsapps/web/utils/ai/assistant/process-user-request.tsapps/web/app/api/google/webhook/process-history.tsapps/web/utils/reply-tracker/draft-tracking.tsapps/web/utils/webhook/validate-webhook-account.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/route.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/resend/summary/route.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/cold-email/is-cold-email.tsapps/web/utils/ai/assistant/chat.tsapps/web/utils/ai/report/fetch.tsapps/web/utils/outlook/message.tsapps/web/utils/assistant/process-assistant-email.tsapps/web/app/api/resend/digest/route.tsapps/web/utils/email/google.tsapps/web/utils/ai/choose-rule/ai-choose-args.tsapps/web/utils/ai/assistant/process-user-request.tsapps/web/app/api/google/webhook/process-history.tsapps/web/utils/reply-tracker/draft-tracking.tsapps/web/utils/webhook/validate-webhook-account.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/route.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/resend/summary/route.ts
apps/web/utils/ai/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/llm.mdc)
apps/web/utils/ai/**/*.{ts,tsx}: Place main LLM feature implementations under apps/web/utils/ai/
LLM feature functions should follow the provided TypeScript pattern (separate system/user prompts, use createGenerateObject, Zod schema validation, early validation, return result.object)
Keep system prompts and user prompts separate
System prompt should define the LLM's role and task specifications
User prompt should contain the actual data and context
Always define a Zod schema for response validation
Make Zod schemas as specific as possible to guide LLM output
Use descriptive scoped loggers for each feature
Log inputs and outputs with appropriate log levels and include relevant context
Implement early returns for invalid inputs
Use proper error types and logging for failures
Implement fallbacks for AI failures
Add retry logic for transient failures using withRetry
Use XML-like tags to structure data in prompts
Remove excessive whitespace and truncate long inputs in prompts
Format prompt data consistently across similar functions
Use TypeScript types for all parameters and return values in LLM features
Define clear interfaces for complex input/output structures in LLM features
Files:
apps/web/utils/ai/assistant/chat.tsapps/web/utils/ai/report/fetch.tsapps/web/utils/ai/choose-rule/ai-choose-args.tsapps/web/utils/ai/assistant/process-user-request.ts
apps/web/utils/{ai,llms}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/llm.mdc)
Keep related AI functions co-located and extract common patterns into utilities; document complex AI logic with clear comments
Files:
apps/web/utils/ai/assistant/chat.tsapps/web/utils/ai/report/fetch.tsapps/web/utils/ai/choose-rule/ai-choose-args.tsapps/web/utils/ai/assistant/process-user-request.ts
apps/web/app/**
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
NextJS app router structure with (app) directory
Files:
apps/web/app/api/resend/digest/route.tsapps/web/app/api/google/webhook/process-history.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/route.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/resend/summary/route.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/resend/digest/route.tsapps/web/app/api/google/webhook/route.tsapps/web/app/api/resend/summary/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/resend/digest/route.tsapps/web/app/api/google/webhook/route.tsapps/web/app/api/resend/summary/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/resend/digest/route.tsapps/web/app/api/google/webhook/process-history.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/route.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/resend/summary/route.ts
🧠 Learnings (1)
📚 Learning: 2025-09-17T22:05:28.646Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-09-17T22:05:28.646Z
Learning: Applies to apps/web/utils/ai/**/*.{ts,tsx} : Use descriptive scoped loggers for each feature
Applied to files:
apps/web/utils/cold-email/is-cold-email.tsapps/web/utils/ai/choose-rule/ai-choose-args.tsapps/web/utils/ai/assistant/process-user-request.ts
🧬 Code graph analysis (14)
apps/web/utils/cold-email/is-cold-email.ts (1)
apps/web/utils/logger.ts (1)
createScopedLogger(17-80)
apps/web/utils/ai/assistant/chat.ts (1)
apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)
apps/web/utils/ai/report/fetch.ts (1)
apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)
apps/web/utils/outlook/message.ts (1)
apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)
apps/web/utils/assistant/process-assistant-email.ts (2)
apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)apps/web/utils/logger.ts (2)
createScopedLogger(17-80)Logger(5-5)
apps/web/app/api/resend/digest/route.ts (1)
apps/web/utils/logger.ts (2)
createScopedLogger(17-80)Logger(5-5)
apps/web/utils/email/google.ts (3)
apps/web/utils/email/microsoft.ts (1)
forwardEmail(411-416)apps/web/utils/gmail/mail.ts (1)
forwardEmail(176-231)apps/web/utils/outlook/mail.ts (1)
forwardEmail(101-160)
apps/web/utils/ai/choose-rule/ai-choose-args.ts (1)
apps/web/utils/logger.ts (1)
createScopedLogger(17-80)
apps/web/utils/ai/assistant/process-user-request.ts (2)
apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)apps/web/utils/logger.ts (2)
createScopedLogger(17-80)Logger(5-5)
apps/web/app/api/google/webhook/process-history.ts (4)
apps/web/utils/logger.ts (1)
Logger(5-5)apps/web/utils/email/google.ts (1)
processHistory(865-885)apps/web/app/api/google/webhook/process-history-item.ts (1)
processHistoryItem(9-62)apps/web/utils/webhook/process-history-item.ts (1)
processHistoryItem(26-216)
apps/web/utils/reply-tracker/draft-tracking.ts (1)
apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)
apps/web/utils/webhook/validate-webhook-account.ts (2)
apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)apps/web/utils/logger.ts (1)
Logger(5-5)
apps/web/app/api/google/webhook/process-history-item.ts (2)
apps/web/utils/logger.ts (1)
Logger(5-5)apps/web/app/api/google/webhook/process-label-removed-event.ts (1)
handleLabelRemovedEvent(22-96)
apps/web/app/api/google/webhook/route.ts (2)
apps/web/utils/logger.ts (1)
createScopedLogger(17-80)apps/web/app/api/google/webhook/process-history.ts (1)
processHistoryForUser(19-153)
🪛 GitHub Actions: Run Tests
apps/web/app/api/google/webhook/process-history-item.ts
[error] 51-55: Cannot read properties of undefined (reading 'info') on logger usage in process-history-item. Tests fail when skipping/processing history due to missing logger implementation.
apps/web/app/api/google/webhook/process-label-removed-event.ts
[error] 43-43: Cannot read properties of undefined (reading 'info') on logger usage in process-label-removed-event. Tests fail when processing label removals due to missing logger implementation.
🔇 Additional comments (17)
apps/web/utils/ai/report/fetch.ts (2)
71-76: LGTM! Structured logging improvement.The refactor from an interpolated string to a static message with a
sourceNamefield improves log parsing and reduces the risk of exposing sensitive data in log messages. All relevant context is preserved in structured fields.
78-82: LGTM! Consistent structured logging for errors.The error log follows the same structured logging pattern as the info log, ensuring consistency. Including both the error and
sourceConfigprovides good debugging context while keeping the message static.apps/web/utils/assistant/process-assistant-email.ts (5)
2-2: LGTM: Logger import follows guidelines.The import of
createScopedLoggerandLoggertype is correct and aligns with the coding guidelines.
24-28: LGTM: Proper per-call logger initialization.The logger is correctly initialized using
createScopedLoggerwith appropriate context fields via.with(). This follows the coding guidelines: "Typically add the logger initialization at the top of the file when usingcreateScopedLogger" and "Only use.with()on a logger instance within a specific function, not for a global logger."
30-42: LGTM: Logger correctly threaded through processing flow.The logger is properly passed to both
processAssistantEmailInternalandwithProcessingLabels, ensuring consistent logging throughout the processing pipeline.
50-51: LGTM: Function signature correctly updated.The function signature properly extends
ProcessAssistantEmailArgswith theloggerparameter using TypeScript's intersection type.
252-253: LGTM: Logger parameter added to helper function.The
withProcessingLabelsfunction signature is correctly updated to accept and use the logger parameter, ensuring all label operations use the same scoped logger.apps/web/utils/ai/assistant/process-user-request.ts (3)
4-4: LGTM! Logger initialization follows coding guidelines.The scoped logger is correctly initialized using
createScopedLoggerwith.with()to add contextual fields specific to this function call. This approach properly replaces the previous module-level logger pattern and ensures each invocation has its own scoped logging context.Based on coding guidelines.
Also applies to: 53-59
391-391: LGTM! Logger correctly threaded through extracted function.The logger is appropriately passed as a parameter to
getUpdateCategoryToolsince this function is extracted outside theprocessUserRequestclosure. The typing is correct and the logger is used consistently within the function body.Also applies to: 625-625, 630-630
582-582: LGTM! Simplified error logging is cleaner.The error logging has been appropriately simplified by removing the
loggerOptionsspread pattern. Since the logger already has contextual fields attached via.with(), directly passing the error message is sufficient and cleaner.apps/web/utils/outlook/message.ts (1)
95-96: LGTM! Structured logging improves observability.Moving
folderNamefrom an interpolated message to a separate context field follows structured logging best practices, making logs more queryable and machine-readable.apps/web/utils/ai/choose-rule/ai-choose-args.ts (3)
64-68: Logger pattern correctly implemented.The per-call scoped logger creation with contextual fields (email, ruleId, ruleName) follows the coding guidelines and aligns with the PR's objective to thread typed loggers through the codebase. The context is properly added via
.with()within the function scope.
70-81: Log statements appropriately simplified.The log statements now rely on the contextual information from the scoped logger, making them more concise while maintaining full debugging context. This is a clean improvement over the previous approach with inline payload objects.
115-115: Error logging appropriately includes response details.Including the
aiResponsein the warning log is appropriate for this error path, as it provides necessary context for debugging why no tool call was found. The contextual logger fields already provide the email and rule information.apps/web/utils/cold-email/is-cold-email.ts (1)
35-39: Logger creation follows best practices.The scoped logger is correctly initialized inside the function with contextual fields. This aligns with the coding guidelines for using
createScopedLoggerwith.with()in functions.Based on coding guidelines.
apps/web/app/api/google/webhook/route.ts (1)
14-41: Scoped webhook logger wiring looks solidPer-request logger creation and context scoping flow cleanly into
processHistoryForUser, matching the new logging contract. Nice upgrade.apps/web/app/api/google/webhook/process-history.ts (1)
190-204: Per-event scoped logging is a nice improvementCreating a per-item scope via
logger.withbefore delegating keeps downstream logs precise without losing the parent context—exactly what we want here.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
apps/web/utils/gmail/message.ts (2)
91-97: Consider using consistent arrow function syntax.The retry wrapper is correctly applied. However, for consistency with
getMessage(lines 72-80), consider using anasyncarrow function:- const response = await withGmailRetry(() => - gmail.users.messages.list({ + const response = await withGmailRetry(async () => { + return gmail.users.messages.list({ userId: "me", q: `rfc822msgid:${cleanMessageId}`, maxResults: 1, - }), - ); + }); + });Note: Both styles are functionally equivalent, but consistency improves maintainability.
272-280: Consider using consistent arrow function syntax.The retry wrapper is correctly applied. However, for consistency with
getMessage(lines 72-80), consider using anasyncarrow function:- const messages = await withGmailRetry(() => - gmail.users.messages.list({ + const messages = await withGmailRetry(async () => { + return gmail.users.messages.list({ userId: "me", maxResults: options.maxResults, q: options.query, pageToken: options.pageToken, labelIds: options.labelIds, - }), - ); + }); + });Note: Both styles are functionally equivalent, but consistency improves maintainability.
apps/web/app/api/watch/controller.ts (1)
52-76: Consider wrappingcleanupInvalidTokensin try-catch to prevent error masking.The refactored error handling correctly identifies permanent auth failures using exact string matching. However, if
cleanupInvalidTokensitself throws an error (e.g., Prisma operation fails), the outer catch block will capture it and callcaptureException, which may be confusing since it will report the cleanup error rather than the original watch failure.Additionally, the exact string matching at lines 53-55 is fragile—if error messages change slightly, auth failures may go undetected and tokens won't be cleaned up.
Apply this diff to wrap the cleanup call:
if (isInsufficientPermissions || isInvalidGrant) { logger.warn("Auth failure while watching inbox - cleaning up tokens", { emailAccountId, providerName: provider.name, error: errorMessage, }); - await cleanupInvalidTokens({ - emailAccountId, - reason: isInvalidGrant ? "invalid_grant" : "insufficient_permissions", - logger, - }); + try { + await cleanupInvalidTokens({ + emailAccountId, + reason: isInvalidGrant ? "invalid_grant" : "insufficient_permissions", + logger, + }); + } catch (cleanupError) { + logger.error("Failed to cleanup invalid tokens", { + emailAccountId, + cleanupError, + }); + captureException(cleanupError); + } } else {Consider using
.includes()for more resilient error detection if messages vary:const isInsufficientPermissions = errorMessage.includes("insufficient authentication scopes"); const isInvalidGrant = errorMessage.includes("invalid_grant");apps/web/utils/auth/cleanup-invalid-tokens.ts (1)
10-41: Consider adding error handling within the function.The function correctly implements token cleanup logic and follows the scoped logger pattern. However, it lacks internal error handling for Prisma operations. If either
findUniqueorupdatefails, the error propagates to the caller without context that it originated from the cleanup operation.Additionally, there's no verification that the
accountexists before attempting to update it (line 31). If theemailAccountis found but the associatedaccountrecord is missing, the update will throw an error.Apply this diff to add error handling:
export async function cleanupInvalidTokens({ emailAccountId, reason, logger, }: { emailAccountId: string; reason: "invalid_grant" | "insufficient_permissions"; logger: Logger; }) { - logger.info("Cleaning up invalid tokens", { reason }); - - const emailAccount = await prisma.emailAccount.findUnique({ - where: { id: emailAccountId }, - select: { accountId: true }, - }); - - if (!emailAccount) { - logger.warn("Email account not found"); - return; - } - - await prisma.account.update({ - where: { id: emailAccount.accountId }, - data: { - access_token: null, - refresh_token: null, - expires_at: null, - }, - }); - - logger.info("Tokens cleared - user must re-authenticate", { reason }); + try { + logger.info("Cleaning up invalid tokens", { reason }); + + const emailAccount = await prisma.emailAccount.findUnique({ + where: { id: emailAccountId }, + select: { accountId: true }, + }); + + if (!emailAccount) { + logger.warn("Email account not found"); + return; + } + + await prisma.account.update({ + where: { id: emailAccount.accountId }, + data: { + access_token: null, + refresh_token: null, + expires_at: null, + }, + }); + + logger.info("Tokens cleared - user must re-authenticate", { reason }); + } catch (error) { + logger.error("Failed to cleanup invalid tokens", { + emailAccountId, + reason, + error, + }); + throw error; + } }apps/web/utils/webhook/validate-webhook-account.ts (1)
105-140: Consider consistency in log message detail.The log messages show inconsistent detail levels:
- Line 105-109: "Account not premium" includes context (lemonSqueezyRenewsAt, stripeSubscriptionStatus)
- Line 121: "Does not have ai access" has no context
- Line 132: "Has no rules enabled" has no context
- Line 140: "Missing access or refresh token" has no context
While the reduced verbosity aligns with the PR's goals, consider whether minimal context (e.g.,
{ hasAiAccess: false }) would aid debugging without adding noise.apps/web/app/api/google/webhook/process-label-removed-event.ts (1)
146-149: Add biome-ignore comment for consistency.The logger parameter reassignment pattern matches line 36 in
apps/web/app/api/google/webhook/process-history.ts, which includes abiome-ignore lint/style/noParameterAssigncomment. For consistency, add the same comment here.+ // biome-ignore lint/style/noParameterAssign: allowed for logging logger = logger.with({ labelName, sender, });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/web/app/api/google/webhook/process-history-item.test.ts(6 hunks)apps/web/app/api/google/webhook/process-history-item.ts(3 hunks)apps/web/app/api/google/webhook/process-history.ts(6 hunks)apps/web/app/api/google/webhook/process-label-removed-event.test.ts(9 hunks)apps/web/app/api/google/webhook/process-label-removed-event.ts(5 hunks)apps/web/app/api/outlook/webhook/process-history.ts(1 hunks)apps/web/app/api/watch/controller.ts(2 hunks)apps/web/utils/auth/cleanup-invalid-tokens.ts(1 hunks)apps/web/utils/gmail/message.ts(4 hunks)apps/web/utils/webhook/validate-webhook-account.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
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/watch/controller.tsapps/web/utils/gmail/message.tsapps/web/app/api/google/webhook/process-history.tsapps/web/utils/auth/cleanup-invalid-tokens.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/process-history-item.test.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/google/webhook/process-label-removed-event.test.tsapps/web/utils/webhook/validate-webhook-account.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/watch/controller.tsapps/web/app/api/google/webhook/process-history.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/process-history-item.test.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/google/webhook/process-label-removed-event.test.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/watch/controller.tsapps/web/utils/gmail/message.tsapps/web/app/api/google/webhook/process-history.tsapps/web/utils/auth/cleanup-invalid-tokens.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/process-history-item.test.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/google/webhook/process-label-removed-event.test.tsapps/web/utils/webhook/validate-webhook-account.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/watch/controller.tsapps/web/utils/gmail/message.tsapps/web/app/api/google/webhook/process-history.tsapps/web/utils/auth/cleanup-invalid-tokens.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/process-history-item.test.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/google/webhook/process-label-removed-event.test.tsapps/web/utils/webhook/validate-webhook-account.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/watch/controller.tsapps/web/utils/gmail/message.tsapps/web/app/api/google/webhook/process-history.tsapps/web/utils/auth/cleanup-invalid-tokens.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/process-history-item.test.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/google/webhook/process-label-removed-event.test.tsapps/web/utils/webhook/validate-webhook-account.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/watch/controller.tsapps/web/app/api/google/webhook/process-history.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/process-history-item.test.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/google/webhook/process-label-removed-event.test.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/watch/controller.tsapps/web/utils/gmail/message.tsapps/web/app/api/google/webhook/process-history.tsapps/web/utils/auth/cleanup-invalid-tokens.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/process-history-item.test.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/google/webhook/process-label-removed-event.test.tsapps/web/utils/webhook/validate-webhook-account.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/watch/controller.tsapps/web/utils/gmail/message.tsapps/web/app/api/google/webhook/process-history.tsapps/web/utils/auth/cleanup-invalid-tokens.tsapps/web/app/api/google/webhook/process-history-item.tsapps/web/app/api/google/webhook/process-history-item.test.tsapps/web/app/api/google/webhook/process-label-removed-event.tsapps/web/app/api/google/webhook/process-label-removed-event.test.tsapps/web/utils/webhook/validate-webhook-account.tsapps/web/app/api/outlook/webhook/process-history.ts
apps/web/utils/gmail/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/gmail-api.mdc)
Keep provider-specific implementation details isolated in the appropriate utils subfolder (e.g., 'apps/web/utils/gmail/')
Files:
apps/web/utils/gmail/message.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/gmail/message.tsapps/web/utils/auth/cleanup-invalid-tokens.tsapps/web/utils/webhook/validate-webhook-account.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/gmail/message.tsapps/web/utils/auth/cleanup-invalid-tokens.tsapps/web/utils/webhook/validate-webhook-account.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/app/api/google/webhook/process-history-item.test.tsapps/web/app/api/google/webhook/process-label-removed-event.test.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/app/api/google/webhook/process-history-item.test.tsapps/web/app/api/google/webhook/process-label-removed-event.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
**/*.test.{ts,tsx}: Use Vitest (vitest) as the testing framework
Colocate tests next to the file under test (e.g., dir/format.ts with dir/format.test.ts)
In tests, mock theserver-onlymodule withvi.mock("server-only", () => ({}));
When testing code that uses Prisma, mock it withvi.mock("@/utils/prisma")and use the mock from@/utils/__mocks__/prisma
Use provided helpers for mocks: import{ getEmail, getEmailAccount, getRule }from@/__tests__/helpers
Each test should be independent
Use descriptive test names
Mock external dependencies in tests
Clean up mocks between tests (e.g.,vi.clearAllMocks()inbeforeEach)
Avoid testing implementation details; focus on observable behavior
Do not mock the Logger
Files:
apps/web/app/api/google/webhook/process-history-item.test.tsapps/web/app/api/google/webhook/process-label-removed-event.test.ts
🧠 Learnings (2)
📚 Learning: 2025-06-23T12:26:53.882Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/prisma.mdc:0-0
Timestamp: 2025-06-23T12:26:53.882Z
Learning: In this project, Prisma should be imported using 'import prisma from "@/utils/prisma";' in TypeScript files.
Applied to files:
apps/web/app/api/watch/controller.ts
📚 Learning: 2025-09-20T18:24:34.280Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-09-20T18:24:34.280Z
Learning: Applies to **/*.test.{ts,tsx} : Use provided helpers for mocks: import `{ getEmail, getEmailAccount, getRule }` from `@/__tests__/helpers`
Applied to files:
apps/web/app/api/google/webhook/process-history-item.test.ts
🧬 Code graph analysis (10)
apps/web/app/api/watch/controller.ts (2)
apps/web/utils/auth/cleanup-invalid-tokens.ts (1)
cleanupInvalidTokens(10-41)apps/web/utils/error.ts (1)
captureException(63-75)
apps/web/utils/gmail/message.ts (2)
apps/web/utils/gmail/retry.ts (1)
withGmailRetry(10-102)apps/web/utils/types.ts (1)
MessageWithPayload(39-49)
apps/web/app/api/google/webhook/process-history.ts (5)
apps/web/utils/logger.ts (1)
Logger(5-5)apps/web/utils/webhook/validate-webhook-account.ts (1)
getWebhookEmailAccount(8-67)apps/web/utils/email/google.ts (1)
processHistory(865-885)apps/web/app/api/google/webhook/process-history-item.ts (1)
processHistoryItem(9-60)apps/web/utils/webhook/process-history-item.ts (1)
processHistoryItem(26-216)
apps/web/utils/auth/cleanup-invalid-tokens.ts (2)
apps/web/utils/logger.ts (1)
Logger(5-5)apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)
apps/web/app/api/google/webhook/process-history-item.ts (3)
apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)apps/web/utils/logger.ts (1)
Logger(5-5)apps/web/app/api/google/webhook/process-label-removed-event.ts (1)
handleLabelRemovedEvent(26-129)
apps/web/app/api/google/webhook/process-history-item.test.ts (3)
apps/web/utils/logger.ts (1)
createScopedLogger(17-80)apps/web/app/api/google/webhook/process-history-item.ts (1)
processHistoryItem(9-60)apps/web/utils/webhook/process-history-item.ts (1)
processHistoryItem(26-216)
apps/web/app/api/google/webhook/process-label-removed-event.ts (2)
apps/web/utils/error.ts (2)
isGmailRateLimitExceededError(102-104)isGmailQuotaExceededError(106-108)apps/web/utils/rule/consts.ts (1)
getRuleLabel(122-124)
apps/web/app/api/google/webhook/process-label-removed-event.test.ts (2)
apps/web/utils/logger.ts (1)
createScopedLogger(17-80)apps/web/app/api/google/webhook/process-label-removed-event.ts (1)
handleLabelRemovedEvent(26-129)
apps/web/utils/webhook/validate-webhook-account.ts (2)
apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)apps/web/utils/logger.ts (1)
Logger(5-5)
apps/web/app/api/outlook/webhook/process-history.ts (2)
apps/web/utils/webhook/validate-webhook-account.ts (1)
getWebhookEmailAccount(8-67)apps/web/app/api/outlook/webhook/logger.ts (1)
logger(3-3)
🔇 Additional comments (15)
apps/web/utils/gmail/message.ts (2)
19-19: LGTM: Import added for retry wrapper.The import of
withGmailRetryis correctly added to support the retry logic for Gmail API calls.
72-80: LGTM: Retry logic correctly applied.The Gmail API call is properly wrapped with
withGmailRetryto handle rate limiting. The async arrow function correctly awaits the API call and returns the typed data.apps/web/app/api/watch/controller.ts (1)
4-4: LGTM!The import follows the project's path alias conventions and correctly references the new utility function.
apps/web/utils/auth/cleanup-invalid-tokens.ts (1)
1-3: LGTM!The imports follow project conventions: Prisma is imported using the project's standard path, and the Logger type is correctly imported.
apps/web/app/api/google/webhook/process-label-removed-event.test.ts (1)
8-10: LGTM! Logger setup follows testing guidelines.The test-scoped logger is correctly initialized at the module level and passed to all test invocations, aligning with the new function signature that accepts a logger parameter.
apps/web/app/api/outlook/webhook/process-history.ts (1)
19-24: LGTM! Logger properly threaded through validation.The logger is correctly passed to
getWebhookEmailAccount, maintaining the per-request logging context established in the broader webhook processing flow.apps/web/app/api/google/webhook/process-history-item.test.ts (2)
12-14: LGTM! Test logger correctly initialized.The test-scoped logger follows the established pattern and is appropriately passed to all function invocations under test.
92-92: LGTM! Simplified mock is appropriate for tests.Using a hardcoded "Cold Email" string in the mock simplifies the test setup and is acceptable since the focus is on testing the logger propagation and event processing logic.
apps/web/app/api/google/webhook/process-history-item.ts (2)
7-18: LGTM! Logger parameter properly integrated.The logger is correctly added to the function signature and will be threaded through to downstream processing handlers.
36-43: LGTM! Logger correctly propagated to label event handler.The logger is properly passed through to
handleLabelRemovedEvent, maintaining the logging context for label removal processing.apps/web/utils/webhook/validate-webhook-account.ts (1)
10-66: LGTM! Logger parameter correctly integrated.The logger is properly added to the function signature and used for error logging when the account is not found, maintaining consistent error handling patterns.
apps/web/app/api/google/webhook/process-label-removed-event.ts (2)
35-53: LGTM! Logger integration and enhanced logging.The logger parameter is correctly integrated, and the updated logging provides appropriate context for label removal processing while reducing verbosity.
57-87: LGTM! Improved error handling for Gmail API issues.The enhanced error handling correctly distinguishes between expected errors (not found, rate limit, quota exceeded) and unexpected errors, with appropriate logging for each case.
apps/web/app/api/google/webhook/process-history.ts (2)
25-36: LGTM! Logger properly integrated with scoped context.The logger parameter is correctly added and augmented with email and emailAccountId context using
logger.with(). The biome-ignore comment appropriately acknowledges the intentional parameter reassignment for logging context.
190-196: LGTM! Excellent per-event logging pattern.Creating a per-event logger with messageId and threadId context is a clean approach that maintains detailed traceability while keeping the logging architecture modular. This pattern effectively balances observability with maintainability.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/utils/webhook/validate-webhook-account.test.ts(2 hunks)
🧰 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/utils/webhook/validate-webhook-account.test.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/webhook/validate-webhook-account.test.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/webhook/validate-webhook-account.test.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/webhook/validate-webhook-account.test.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/utils/webhook/validate-webhook-account.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/webhook/validate-webhook-account.test.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/webhook/validate-webhook-account.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/utils/webhook/validate-webhook-account.test.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/webhook/validate-webhook-account.test.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/utils/webhook/validate-webhook-account.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
**/*.test.{ts,tsx}: Use Vitest (vitest) as the testing framework
Colocate tests next to the file under test (e.g., dir/format.ts with dir/format.test.ts)
In tests, mock theserver-onlymodule withvi.mock("server-only", () => ({}));
When testing code that uses Prisma, mock it withvi.mock("@/utils/prisma")and use the mock from@/utils/__mocks__/prisma
Use provided helpers for mocks: import{ getEmail, getEmailAccount, getRule }from@/__tests__/helpers
Each test should be independent
Use descriptive test names
Mock external dependencies in tests
Clean up mocks between tests (e.g.,vi.clearAllMocks()inbeforeEach)
Avoid testing implementation details; focus on observable behavior
Do not mock the Logger
Files:
apps/web/utils/webhook/validate-webhook-account.test.ts
🧠 Learnings (1)
📚 Learning: 2025-09-20T18:24:34.280Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-09-20T18:24:34.280Z
Learning: Applies to **/*.test.{ts,tsx} : Do not mock the Logger
Applied to files:
apps/web/utils/webhook/validate-webhook-account.test.ts
🧬 Code graph analysis (1)
apps/web/utils/webhook/validate-webhook-account.test.ts (1)
apps/web/utils/logger.ts (1)
Logger(5-5)
🔇 Additional comments (1)
apps/web/utils/webhook/validate-webhook-account.test.ts (1)
90-257: Test structure and coverage look good.The test cases comprehensively cover different validation scenarios including null account, non-premium users, missing AI access, no automation rules, and missing tokens. The assertions appropriately verify both the success/failure status and the unwatchEmails behavior.
Once the Logger mock is replaced with a real logger instance (as noted in the previous comment), these tests will align with the coding guidelines.
| const mockLogger = { | ||
| error: vi.fn(), | ||
| info: vi.fn(), | ||
| warn: vi.fn(), | ||
| trace: vi.fn(), | ||
| }; | ||
| with: vi.fn().mockReturnThis(), | ||
| } as unknown as Logger; |
There was a problem hiding this comment.
Remove the Logger mock and use a real logger instance.
The mockLogger violates the coding guideline that explicitly states: "Do not mock the Logger" for test files. Instead of mocking, create a real logger instance using createScopedLogger from @/utils/logger.
Based on learnings.
Replace the mock with a real logger:
+import { createScopedLogger } from "@/utils/logger";
+
describe("validateWebhookAccount", () => {
- const mockLogger = {
- error: vi.fn(),
- info: vi.fn(),
- warn: vi.fn(),
- trace: vi.fn(),
- with: vi.fn().mockReturnThis(),
- } as unknown as Logger;
+ const logger = createScopedLogger("validateWebhookAccount.test");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const mockLogger = { | |
| error: vi.fn(), | |
| info: vi.fn(), | |
| warn: vi.fn(), | |
| trace: vi.fn(), | |
| }; | |
| with: vi.fn().mockReturnThis(), | |
| } as unknown as Logger; | |
| // Add this at the top of the file | |
| import { createScopedLogger } from "@/utils/logger"; | |
| describe("validateWebhookAccount", () => { | |
| - const mockLogger = { | |
| - error: vi.fn(), | |
| - info: vi.fn(), | |
| - warn: vi.fn(), | |
| - trace: vi.fn(), | |
| - with: vi.fn().mockReturnThis(), | |
| const logger = createScopedLogger("validateWebhookAccount.test"); | |
| // ...the rest of your test cases, now using `logger` instead of `mockLogger`... | |
| }); |
Summary by CodeRabbit
Bug Fixes
Refactor
Chores