Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughConfiguration and environment setup for OpenAI zero-data retention feature introduced, along with webhook URL handling refinements. OpenAI provider options updated to safely enforce data non-storage when flag enabled. Base URL selection simplified in webhook watch logic. Version bumped to v2.21.37. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Key areas requiring attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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
🧹 Nitpick comments (2)
apps/web/.env.example (1)
36-36: Add example values and hints for new env vars
WEBHOOK_URLandOPENAI_ZERO_DATA_RETENTIONare documented, but it would help to show expected formats, e.g.:
# WEBHOOK_URL=https://your-ngrok-or-host.com# OPENAI_ZERO_DATA_RETENTION=true # disables OpenAI data storage when enabledThis keeps
.env.exampleconsistent with other entries and clarifies intended usage.Based on learnings, env examples should include sample values and unquoted booleans.
Also applies to: 62-62
apps/web/utils/outlook/watch.ts (1)
8-8: Custom webhook base URL handling looks good; consider optional guardUsing
env.WEBHOOK_URL || env.NEXT_PUBLIC_BASE_URLcleanly supports an override for the Outlook webhook while preserving the existing base URL fallback. You might optionally add an explicit check (or logger) if both are missing to fail fast with a clearer error thannew URL()throwing, but the current behavior is otherwise sound.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/.env.example(2 hunks)apps/web/utils/llms/model.ts(1 hunks)apps/web/utils/outlook/watch.ts(1 hunks)turbo.json(1 hunks)version.txt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
turbo.json
📄 CodeRabbit inference engine (.cursor/rules/environment-variables.mdc)
Add new environment variables to
turbo.jsonundertasks.build.envas a global dependency for the build task
Files:
turbo.json
!(pages/_document).{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't use the next/head module in pages/_document.js on Next.js projects
Files:
turbo.jsonversion.txtapps/web/utils/llms/model.tsapps/web/.env.exampleapps/web/utils/outlook/watch.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Use@/path aliases for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Follow consistent naming conventions using PascalCase for components
Centralize shared types in dedicated type filesImport specific lodash functions rather than entire lodash library to minimize bundle size (e.g.,
import groupBy from 'lodash/groupBy')
Files:
apps/web/utils/llms/model.tsapps/web/utils/outlook/watch.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)
**/*.{ts,tsx}: For API GET requests to server, use theswrpackage
Useresult?.serverErrorwithtoastErrorfrom@/components/Toastfor error handling in async operations
**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls
**/*.{ts,tsx}: For early access feature flags, create hooks using the naming conventionuse[FeatureName]Enabledthat return a boolean fromuseFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming conventionuse[FeatureName]Variantthat define variant types, useuseFeatureFlagVariantKey()with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g.,inbox-cleaner,pricing-options-2)
Always define types for A/B test variant flags (e.g.,type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
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 TypeScript namespaces
Don't use non-null assertions with the!postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Useas constinstead of literal types and type annotations
Use eitherT[]orArray<T>consistently
Initialize each enum member value explicitly
Useexport typefor types
Use `impo...
Files:
apps/web/utils/llms/model.tsapps/web/utils/outlook/watch.ts
apps/web/{utils/ai,utils/llms,__tests__}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm.mdc)
LLM-related code must be organized in specific directories:
apps/web/utils/ai/for main implementations,apps/web/utils/llms/for core utilities and configurations, andapps/web/__tests__/for LLM-specific tests
Files:
apps/web/utils/llms/model.ts
apps/web/utils/llms/{index,model}.ts
📄 CodeRabbit inference engine (.cursor/rules/llm.mdc)
Core LLM functionality must be defined in
utils/llms/index.ts, model definitions and configurations inutils/llms/model.ts, and usage tracking inutils/usage.ts
Files:
apps/web/utils/llms/model.ts
**/{server,api,actions,utils}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/{server,api,actions,utils}/**/*.ts: UsecreateScopedLoggerfrom "@/utils/logger" for logging in backend code
Add thecreateScopedLoggerinstantiation at the top of the file with an appropriate scope name
Use.with()method to attach context variables only within specific functions, not on global loggers
For large functions with reused variables, usecreateScopedLogger().with()to attach context once and reuse the logger without passing variables repeatedly
Files:
apps/web/utils/llms/model.tsapps/web/utils/outlook/watch.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)
Always import Prisma enums from
@/generated/prisma/enumsinstead of@/generated/prisma/clientto avoid Next.js bundling errors in client componentsImport Prisma using the project's centralized utility:
import prisma from '@/utils/prisma'
Files:
apps/web/utils/llms/model.tsapps/web/utils/outlook/watch.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/security.mdc)
**/*.ts: ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access
Always validate that resources belong to the authenticated user before performing operations, using ownership checks in WHERE clauses or relationships
Always validate all input parameters for type, format, and length before using them in database queries
Use SafeError for error responses to prevent information disclosure. Generic error messages should not reveal internal IDs, logic, or resource ownership details
Only return necessary fields in API responses using Prisma'sselectoption. Never expose sensitive data such as password hashes, private keys, or system flags
Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations. AllfindUnique/findFirstcalls MUST include ownership filters
Prevent mass assignment vulnerabilities by explicitly whitelisting allowed fields in update operations instead of accepting all user-provided data
Prevent privilege escalation by never allowing users to modify system fields, ownership fields, or admin-only attributes through user input
AllfindManyqueries MUST be scoped to the user's data by including appropriate WHERE filters to prevent returning data from other users
Use Prisma relationships for access control by leveraging nested where clauses (e.g.,emailAccount: { id: emailAccountId }) to validate ownership
Files:
apps/web/utils/llms/model.tsapps/web/utils/outlook/watch.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Usenext/imagepackage for images
For API GET requests to server, use theswrpackage with hooks likeuseSWRto fetch data
For text inputs, use theInputcomponent withregisterPropsfor form integration and error handling
Files:
apps/web/utils/llms/model.tsapps/web/utils/outlook/watch.ts
**/*.{tsx,ts,css}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Implement responsive design with Tailwind CSS using a mobile-first approach
Files:
apps/web/utils/llms/model.tsapps/web/utils/outlook/watch.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useaccessKeyattribute on any HTML element
Don't setaria-hidden="true"on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like<marquee>or<blink>
Only use thescopeprop on<th>elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assigntabIndexto non-interactive HTML elements
Don't use positive integers fortabIndexproperty
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include atitleelement for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
AssigntabIndexto non-interactive HTML elements witharia-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include atypeattribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden)
Always include alangattribute on the html element
Always include atitleattribute for iframe elements
AccompanyonClickwith at least one of:onKeyUp,onKeyDown, oronKeyPress
AccompanyonMouseOver/onMouseOutwithonFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...
Files:
apps/web/utils/llms/model.tsapps/web/utils/outlook/watch.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g.,import groupBy from 'lodash/groupBy')
Files:
apps/web/utils/llms/model.tsapps/web/utils/outlook/watch.ts
apps/web/**/{.env.example,env.ts,turbo.json}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
Add environment variables to
.env.example,env.ts, andturbo.json
Files:
apps/web/.env.example
🧠 Learnings (13)
📚 Learning: 2025-11-25T14:36:45.807Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:45.807Z
Learning: Applies to turbo.json : Add new environment variables to `turbo.json` under `tasks.build.env` as a global dependency for the build task
Applied to files:
turbo.json
📚 Learning: 2025-11-25T14:36:43.454Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:43.454Z
Learning: Applies to turbo.json : Add environment variables to `turbo.json` under `tasks.build.env` array to declare build-time dependencies
Applied to files:
turbo.json
📚 Learning: 2025-11-25T14:36:18.416Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-11-25T14:36:18.416Z
Learning: Applies to apps/web/**/{.env.example,env.ts,turbo.json} : Add environment variables to `.env.example`, `env.ts`, and `turbo.json`
Applied to files:
turbo.jsonapps/web/.env.example
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : LLM feature functions must import from `zod` for schema validation, use `createScopedLogger` from `@/utils/logger`, `chatCompletionObject` and `createGenerateObject` from `@/utils/llms`, and import `EmailAccountWithAI` type from `@/utils/llms/types`
Applied to files:
apps/web/utils/llms/model.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/llms/{index,model}.ts : Core LLM functionality must be defined in `utils/llms/index.ts`, model definitions and configurations in `utils/llms/model.ts`, and usage tracking in `utils/usage.ts`
Applied to files:
apps/web/utils/llms/model.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Use TypeScript types for all LLM function parameters and return values, and define clear interfaces for complex input/output structures
Applied to files:
apps/web/utils/llms/model.ts
📚 Learning: 2025-11-25T14:36:45.807Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:45.807Z
Learning: Applies to .env.example : Add new environment variables to `.env.example` with example values
Applied to files:
apps/web/.env.example
📚 Learning: 2025-07-19T15:06:43.730Z
Learnt from: garnertb
Repo: elie222/inbox-zero PR: 580
File: apps/web/.env.example:4-7
Timestamp: 2025-07-19T15:06:43.730Z
Learning: In apps/web/.env.example, boolean environment variables follow an unquoted convention (e.g., LOG_ZOD_ERRORS=true, AUTH_TRUST_HOST=true). Logical grouping of related variables (like auth variables together) is preferred over strict alphabetical ordering for better developer experience.
Applied to files:
apps/web/.env.example
📚 Learning: 2025-11-25T14:36:43.454Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:43.454Z
Learning: Applies to .env.example : Add environment variables to `.env.example` with example values in the format `VARIABLE_NAME=value_example`
Applied to files:
apps/web/.env.example
📚 Learning: 2025-11-25T14:36:45.807Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:45.807Z
Learning: Applies to apps/web/env.ts : Add client-side environment variables to `apps/web/env.ts` under the `experimental__runtimeEnv` object to enable runtime access
Applied to files:
apps/web/.env.example
📚 Learning: 2025-11-25T14:36:45.807Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:45.807Z
Learning: Applies to apps/web/env.ts : Add client-side environment variables to `apps/web/env.ts` under the `client` object with `NEXT_PUBLIC_` prefix and Zod schema validation
Applied to files:
apps/web/.env.example
📚 Learning: 2025-11-25T14:36:43.454Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:43.454Z
Learning: Applies to apps/web/env.ts : For client-side environment variables in `apps/web/env.ts`, prefix them with `NEXT_PUBLIC_` and add them to both the `client` and `experimental__runtimeEnv` sections
Applied to files:
apps/web/.env.example
📚 Learning: 2025-11-25T14:36:45.807Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:45.807Z
Learning: Applies to apps/web/env.ts : Add server-only environment variables to `apps/web/env.ts` under the `server` object with Zod schema validation
Applied to files:
apps/web/.env.example
🧬 Code graph analysis (2)
apps/web/utils/llms/model.ts (2)
apps/web/utils/llms/config.ts (1)
providerOptions(18-29)apps/web/env.ts (1)
env(17-246)
apps/web/utils/outlook/watch.ts (1)
apps/web/env.ts (1)
env(17-246)
⏰ 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). (4)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Review for correctness
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
version.txt (1)
1-1: Version bump aligns with config changesv2.21.37 correctly reflects the new env/config wiring introduced in this PR; no further action needed here.
turbo.json (1)
35-35: OPENAI_ZERO_DATA_RETENTION correctly wired into build envIncluding
OPENAI_ZERO_DATA_RETENTIONintasks.build.envensures builds react to flag changes and matches the env/.env wiring for this feature.Based on learnings, new env vars should be declared under
tasks.build.env.apps/web/utils/llms/model.ts (1)
74-79: Zero‑data‑retention provider options handling is correctUsing
baseOptions = providerOptions ?? {}and overridingopenai.storewhenenv.OPENAI_ZERO_DATA_RETENTIONis true safely covers the undefined case, preserves existing options, and avoids mutating the originalproviderOptionsobject. This matches the intended zero‑data‑retention behavior.
Allow Outlook webhook subscriptions to use
env.WEBHOOK_URLin production by updatingwatch.watchOutlookin watch.tsUpdate base URL selection to prefer
env.WEBHOOK_URL; add safe defaults for Zero Data Retention inutils/llms/model.selectModel; addOPENAI_ZERO_DATA_RETENTIONto env examples and pipeline; bump version.📍Where to Start
Start with the
watch.watchOutlookfunction in watch.ts.📊 Macroscope summarized 7d8313e. 2 files reviewed, 9 issues evaluated, 9 issues filtered, 0 comments posted
🗂️ Filtered Issues
apps/web/utils/llms/model.ts — 0 comments posted, 6 evaluated, 6 filtered
createOpenAI({ apiKey: aiApiKey || env.OPENAI_API_KEY })may pass an undefined API key when neitheraiApiKeynorenv.OPENAI_API_KEYis set. This defers failure to a later network call or causes opaque errors inside the SDK. Validate and fail fast with a clear error when no key is available for the selected provider. [ Out of scope ]createGoogleGenerativeAI({ apiKey: aiApiKey || env.GOOGLE_API_KEY })may receive an undefined API key if neither source is set, leading to runtime or request-time failures. Add explicit validation and throw a clear error when missing. [ Out of scope ]createGroq({ apiKey: aiApiKey || env.GROQ_API_KEY })may be called with an undefined API key if neither is set, causing late failures. Validate and throw early with a clear message when using the GROQ provider without a key. [ Out of scope ]createOpenRouter({ apiKey: aiApiKey || env.OPENROUTER_API_KEY, headers: { ... } })may receive an undefined API key when neither source is set, leading to runtime failures. Add explicit validation for the OpenRouter provider. [ Out of scope ]createGateway({ apiKey: aiGatewayApiKey })may be invoked with an undefined API key if bothaiApiKeyandenv.AI_GATEWAY_API_KEYare unset, leading to runtime errors at request time. Validate before constructing the gateway. [ Out of scope ]region: env.BEDROCK_REGIONmay be undefined, andcredentialProviderreturnsaccessKeyIdandsecretAccessKeytaken fromenv.BEDROCK_ACCESS_KEY!andenv.BEDROCK_SECRET_KEY!using non-null assertions. If any are unset, the Bedrock client will fail later with opaque AWS auth/region errors. Validate and throw a clear error when any required field is missing. [ Out of scope ]apps/web/utils/outlook/watch.ts — 0 comments posted, 3 evaluated, 3 filtered
new URL("/api/outlook/webhook", base)will throw at runtime ifbaseisundefined, empty, or not a valid absolute URL with a scheme. There is no guard ensuringenv.WEBHOOK_URL || env.NEXT_PUBLIC_BASE_URLis present and valid, sowatchOutlookcan synchronously crash before any external call when those env vars are missing or malformed. [ Low confidence ]notificationUrl.protocol = "https:"can yield invalid combinations when thebaseincludes an explicit port (e.g.,http://example.com:80). The resulting URL becomeshttps://example.com:80/..., which commonly fails Graph validation or connectivity for HTTPS. If HTTPS is required, rejecthttpbases or rebuild with default HTTPS origin/port rather than mutating the protocol in place. [ Low confidence ]expirationDateTime: addDays(new Date(), 3).toISOString()sets an expiration at exactly 3 days. For Microsoft Graph mail (/me/messages) subscriptions, the documented maximum is 4230 minutes (~2.94 days). Setting 3 days (4320 minutes) can cause the POST to fail with a validation error, andwithOutlookRetrycannot succeed because the request is deterministically invalid. [ Out of scope ]Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.