Skip to content

Update packages#1061

Merged
elie222 merged 4 commits intostagingfrom
chore/upgrade-packages
Dec 4, 2025
Merged

Update packages#1061
elie222 merged 4 commits intostagingfrom
chore/upgrade-packages

Conversation

@elie222
Copy link
Copy Markdown
Owner

@elie222 elie222 commented Dec 4, 2025

Update packages and set zero data retention for OpenAI by adding env.createEnv server var OPENAI_ZERO_DATA_RETENTION and applying providerOptions.openai.store=false in selectModel when true, and refresh Outlook tokens when expiry is within 600000 ms

Add OPENAI_ZERO_DATA_RETENTION to env.createEnv with boolean coercion and default false; pass providerOptions in selectModel for Provider.OPEN_AI with openai.store=false when the env var is true; refresh Outlook access tokens earlier with a 10-minute buffer; update auth and SDK dependencies and remove a root patch.

📍Where to Start

Start with selectModel in apps/web/utils/llms/model.ts, then review the env var addition in apps/web/env.ts and the Outlook token logic in apps/web/utils/outlook/client.ts.


Macroscope summarized bc11885.

Summary by CodeRabbit

  • New Features

    • Added OpenAI Zero Data Retention configuration option
    • Included OAuth client metadata fields (logo and terms URL) for dynamic registration
  • Improvements

    • Outlook token refresh now triggers earlier with a 10-minute safety buffer
  • Updates

    • Updated core dependencies: Model Context Protocol SDK, auth framework, and email delivery package
  • Chores

    • Removed a patched dependency entry and bumped release version to v2.21.37

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link
Copy Markdown

vercel bot commented Dec 4, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
inbox-zero Ready Ready Preview Dec 4, 2025 4:49am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 4, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Added an OPENAI_ZERO_DATA_RETENTION env flag; adjusted OpenAI provider options to respect that flag; introduced a token refresh buffer for Outlook; updated several dependencies and removed a pnpm patch; bumped version.

Changes

Cohort / File(s) Change Summary
Configuration & Version
apps/web/env.ts, version.txt
Added OPENAI_ZERO_DATA_RETENTION (boolean, optional, default false) to server env schema; bumped version from v2.21.34v2.21.37
LLM Provider Options
apps/web/utils/llms/model.ts
When OPENAI_ZERO_DATA_RETENTION is true, clone providerOptions and set openai.store = false; returned model includes the adjusted providerOptions
Outlook Integration
apps/web/utils/outlook/client.ts
Added TOKEN_REFRESH_BUFFER_MS = 10 * 60 * 1000; token-refresh now requires expiryDate > Date.now() + TOKEN_REFRESH_BUFFER_MS, causing earlier refresh
Dependencies (app)
apps/web/package.json
Updated @modelcontextprotocol/sdk (^1.20.1 → 1.24.0), better-auth (1.3.28 → 1.4.5), nodemailer (7.0.9 → 7.0.11)
Patch Management
package.json
Removed pnpm.patchedDependencies entry that patched better-auth@1.3.28

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Review apps/web/utils/llms/model.ts to ensure providerOptions cloning only occurs when the flag is set and does not mutate shared objects.
  • Verify apps/web/utils/outlook/client.ts that the buffer logic behaves as intended around edge expiry times.
  • Confirm dependency bumps (especially better-auth) don't require retained patch behavior.

Possibly related PRs

Poem

🐇 A nibble, then a tiny hop,

I set retention off — no data drop.
Tokens wake up ten minutes early,
Dependencies polished, version pearly.
Hooray — the inbox stays light and merry!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Update packages' is vague and generic, using a non-descriptive term that does not convey the actual scope of changes, which include environment configuration, provider logic modifications, and token refresh adjustments beyond simple dependency updates. Consider a more descriptive title that captures the primary changes, such as 'Add OpenAI zero data retention flag and update token refresh logic' or 'Add OPENAI_ZERO_DATA_RETENTION env and adjust Outlook token refresh buffer'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/upgrade-packages

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@socket-security
Copy link
Copy Markdown

socket-security bot commented Dec 4, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updated@​modelcontextprotocol/​sdk@​1.20.1 ⏵ 1.24.099100 +2210099100

View full report

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 28df057 and d82566e.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (6)
  • apps/web/env.ts (1 hunks)
  • apps/web/package.json (3 hunks)
  • apps/web/utils/llms/model.ts (1 hunks)
  • apps/web/utils/outlook/client.ts (2 hunks)
  • package.json (0 hunks)
  • version.txt (1 hunks)
💤 Files with no reviewable changes (1)
  • package.json
🧰 Additional context used
📓 Path-based instructions (17)
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 files

Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

Files:

  • apps/web/utils/outlook/client.ts
  • apps/web/utils/llms/model.ts
  • apps/web/env.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)

**/*.{ts,tsx}: For API GET requests to server, use the swr package
Use result?.serverError with toastError from @/components/Toast for 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 convention use[FeatureName]Enabled that return a boolean from useFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming convention use[FeatureName]Variant that define variant types, use useFeatureFlagVariantKey() 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
Use as const instead of literal types and type annotations
Use either T[] or Array<T> consistently
Initialize each enum member value explicitly
Use export type for types
Use `impo...

Files:

  • apps/web/utils/outlook/client.ts
  • apps/web/utils/llms/model.ts
  • apps/web/env.ts
**/{server,api,actions,utils}/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)

**/{server,api,actions,utils}/**/*.ts: Use createScopedLogger from "@/utils/logger" for logging in backend code
Add the createScopedLogger instantiation 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, use createScopedLogger().with() to attach context once and reuse the logger without passing variables repeatedly

Files:

  • apps/web/utils/outlook/client.ts
  • apps/web/utils/llms/model.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)

Always import Prisma enums from @/generated/prisma/enums instead of @/generated/prisma/client to avoid Next.js bundling errors in client components

Import Prisma using the project's centralized utility: import prisma from '@/utils/prisma'

Files:

  • apps/web/utils/outlook/client.ts
  • apps/web/utils/llms/model.ts
  • apps/web/env.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's select option. 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. All findUnique/findFirst calls 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
All findMany queries 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/outlook/client.ts
  • apps/web/utils/llms/model.ts
  • apps/web/env.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Use next/image package for images
For API GET requests to server, use the swr package with hooks like useSWR to fetch data
For text inputs, use the Input component with registerProps for form integration and error handling

Files:

  • apps/web/utils/outlook/client.ts
  • apps/web/utils/llms/model.ts
  • apps/web/env.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/outlook/client.ts
  • apps/web/utils/llms/model.ts
  • apps/web/env.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{js,jsx,ts,tsx}: Don't use accessKey attribute on any HTML element
Don't set aria-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 the scope prop 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 assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex property
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 a title element 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
Assign tabIndex to non-interactive HTML elements with aria-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 a type attribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden)
Always include a lang attribute on the html element
Always include a title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/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/outlook/client.ts
  • apps/web/utils/llms/model.ts
  • apps/web/env.ts
!(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:

  • apps/web/utils/outlook/client.ts
  • apps/web/utils/llms/model.ts
  • version.txt
  • apps/web/package.json
  • apps/web/env.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/outlook/client.ts
  • apps/web/utils/llms/model.ts
  • apps/web/env.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, and apps/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 in utils/llms/model.ts, and usage tracking in utils/usage.ts

Files:

  • apps/web/utils/llms/model.ts
**/package.json

📄 CodeRabbit inference engine (.cursor/rules/installing-packages.mdc)

Use pnpm as the package manager

Files:

  • apps/web/package.json
apps/web/package.json

📄 CodeRabbit inference engine (.cursor/rules/installing-packages.mdc)

Don't install packages in root; install in apps/web workspace instead

Files:

  • apps/web/package.json
apps/web/**/{.env.example,env.ts,turbo.json}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

Add environment variables to .env.example, env.ts, and turbo.json

Files:

  • apps/web/env.ts
apps/web/env.ts

📄 CodeRabbit inference engine (.cursor/rules/environment-variables.mdc)

apps/web/env.ts: Add server-only environment variables to apps/web/env.ts under the server object with Zod schema validation
Add client-side environment variables to apps/web/env.ts under the client object with NEXT_PUBLIC_ prefix and Zod schema validation
Add client-side environment variables to apps/web/env.ts under the experimental__runtimeEnv object to enable runtime access

Files:

  • apps/web/env.ts
{.env.example,apps/web/env.ts}

📄 CodeRabbit inference engine (.cursor/rules/environment-variables.mdc)

Client-side environment variables must be prefixed with NEXT_PUBLIC_

Files:

  • apps/web/env.ts
🧠 Learnings (10)
📚 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: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.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 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.ts
📚 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 : Define environment variables in `apps/web/env.ts` using Zod schema validation, organizing them into `server` and `client` sections

Applied to files:

  • apps/web/env.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 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.ts
📚 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.ts
📚 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:

  • apps/web/env.ts
📚 Learning: 2025-11-25T14:37:56.430Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.430Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Use `describe.runIf(isAiTest)` with environment variable `RUN_AI_TESTS === "true"` to conditionally run LLM tests

Applied to files:

  • apps/web/env.ts
📚 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.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,apps/web/env.ts} : Client-side environment variables must be prefixed with `NEXT_PUBLIC_`

Applied to files:

  • apps/web/env.ts
⏰ 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). (2)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Review for correctness
🔇 Additional comments (4)
version.txt (1)

1-1: Version bump is straightforward

Single-line version update only; no behavioral impact.

apps/web/package.json (1)

45-45: Verify dependency bumps against existing usage

The version updates for:

  • @modelcontextprotocol/sdk1.24.0
  • better-auth1.4.5
  • nodemailer7.0.11

look reasonable, but please confirm:

  • Existing usages (especially around auth flows and email sending) still match the updated APIs.
  • Tests or key auth/email paths have been exercised after removing the better-auth patch override in the root config.

Also applies to: 95-95, 133-133

apps/web/utils/outlook/client.ts (1)

11-12: Confirm expiry units when using TOKEN_REFRESH_BUFFER_MS

The 10‑minute buffer is a good idea, but this condition:

const expiryDate = expiresAt ? expiresAt : null;
if (accessToken && expiryDate && expiryDate > Date.now() + TOKEN_REFRESH_BUFFER_MS) {
  return createOutlookClient(accessToken);
}

assumes expiresAt is in milliseconds (same unit as Date.now() and TOKEN_REFRESH_BUFFER_MS). In this file, new tokens are persisted as:

expires_at: Math.floor(Date.now() / 1000 + tokens.expires_in),

which is seconds since epoch.

If expiresAt is passed through directly from expires_at, the comparison will always treat the token as already within the buffer (or expired), forcing a refresh on every call. Please double‑check that callers multiply by 1000 (or adjust here) so all three values use the same unit.

Also applies to: 103-107

apps/web/env.ts (1)

63-63: Server env flag for OpenAI retention looks good—ensure config is mirrored

OPENAI_ZERO_DATA_RETENTION is correctly added as a server-only boolean with a safe default of false, which keeps existing behavior unchanged and enables the new OpenAI config path.

Please also make sure this variable is:

  • Added to apps/web/.env.example with the usual unquoted boolean style.
  • Included in apps/web/turbo.json if you track envs there.

Based on learnings, these three files should stay in sync for new env vars.

Comment on lines +71 to 80
// When Zero Data Retention is enabled, set store: false to avoid
// "Items are not persisted for Zero Data Retention organizations" errors
// See: https://github.com/vercel/ai/issues/10060
const openAiProviderOptions = env.OPENAI_ZERO_DATA_RETENTION
? {
...providerOptions,
openai: { ...providerOptions?.openai, store: false },
}
: providerOptions;
return {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Prevent runtime error when OPENAI_ZERO_DATA_RETENTION is enabled

providerOptions is optional, but this branch assumes it’s always an object:

const openAiProviderOptions = env.OPENAI_ZERO_DATA_RETENTION
  ? {
      ...providerOptions,
      openai: { ...providerOptions?.openai, store: false },
    }
  : providerOptions;

If providerOptions is undefined (which can happen from callers like selectEconomyModel / selectChatModel), ...providerOptions will throw TypeError: Cannot convert undefined or null to object as soon as OPENAI_ZERO_DATA_RETENTION is set to true.

You can avoid this by normalizing to an empty object before spreading:

-    case Provider.OPEN_AI: {
+    case Provider.OPEN_AI: {
       const modelName = aiModel || "gpt-5.1";
-      // When Zero Data Retention is enabled, set store: false to avoid
-      // "Items are not persisted for Zero Data Retention organizations" errors
-      // See: https://github.com/vercel/ai/issues/10060
-      const openAiProviderOptions = env.OPENAI_ZERO_DATA_RETENTION
-        ? {
-            ...providerOptions,
-            openai: { ...providerOptions?.openai, store: false },
-          }
-        : providerOptions;
+      // When Zero Data Retention is enabled, set store: false to avoid
+      // "Items are not persisted for Zero Data Retention organizations" errors
+      // See: https://github.com/vercel/ai/issues/10060
+      const baseOptions = providerOptions ?? {};
+      const openAiProviderOptions = env.OPENAI_ZERO_DATA_RETENTION
+        ? {
+            ...baseOptions,
+            openai: { ...(baseOptions.openai ?? {}), store: false },
+          }
+        : providerOptions;
       return {
         provider: Provider.OPEN_AI,
         modelName,
         model: createOpenAI({ apiKey: aiApiKey || env.OPENAI_API_KEY })(
           modelName,
         ),
-        providerOptions: openAiProviderOptions,
+        providerOptions: openAiProviderOptions,
         backupModel: getBackupModel(aiApiKey),
       };
     }

This keeps behavior unchanged when the flag is false and avoids crashes when it’s true.

Also applies to: 86-86

🤖 Prompt for AI Agents
In apps/web/utils/llms/model.ts around lines 71 to 80 (also apply same fix at
line 86), the code spreads providerOptions without ensuring it's defined when
OPENAI_ZERO_DATA_RETENTION is true, causing a TypeError if providerOptions is
undefined; fix by normalizing providerOptions to an object before spreading
(e.g., use a fallback {} when spreading and when accessing
providerOptions.openai) so the branch safely constructs openAiProviderOptions
even if providerOptions is undefined.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 7 files

Prompt for AI agents (all 1 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="apps/web/utils/outlook/client.ts">

<violation number="1" location="apps/web/utils/outlook/client.ts:106">
P2: `expiryDate` is stored in seconds but is compared to `Date.now()` (milliseconds), so this condition is always false and forces needless token refreshes. Convert one side so both are in the same units.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

if (
accessToken &&
expiryDate &&
expiryDate > Date.now() + TOKEN_REFRESH_BUFFER_MS
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: expiryDate is stored in seconds but is compared to Date.now() (milliseconds), so this condition is always false and forces needless token refreshes. Convert one side so both are in the same units.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/utils/outlook/client.ts, line 106:

<comment>`expiryDate` is stored in seconds but is compared to `Date.now()` (milliseconds), so this condition is always false and forces needless token refreshes. Convert one side so both are in the same units.</comment>

<file context>
@@ -97,7 +100,11 @@ export const getOutlookClientWithRefresh = async ({
+  if (
+    accessToken &amp;&amp;
+    expiryDate &amp;&amp;
+    expiryDate &gt; Date.now() + TOKEN_REFRESH_BUFFER_MS
+  ) {
     return createOutlookClient(accessToken);
</file context>
Suggested change
expiryDate > Date.now() + TOKEN_REFRESH_BUFFER_MS
expiryDate * 1000 > Date.now() + TOKEN_REFRESH_BUFFER_MS
Fix with Cubic

@@ -97,7 +100,11 @@ export const getOutlookClientWithRefresh = async ({

// Check if token needs refresh
const expiryDate = expiresAt ? expiresAt : null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expiry check mixes seconds (expires_at) with milliseconds (Date.now() + TOKEN_REFRESH_BUFFER_MS). Consider converting expires_at to ms before comparing to avoid unnecessary refreshes.

-  const expiryDate = expiresAt ? expiresAt : null;
+  const expiryDateMs = expiresAt ? expiresAt * 1000 : null;
   if (
-    accessToken &&
-    expiryDate &&
-    expiryDate > Date.now() + TOKEN_REFRESH_BUFFER_MS
+    accessToken &&
+    expiryDateMs &&
+    expiryDateMs > Date.now() + TOKEN_REFRESH_BUFFER_MS
   ) {
     return createOutlookClient(accessToken);
   }

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue on line in apps/web/utils/llms/model.ts:153:

Suggestion: Add defensive checks/defaults in apps/web/utils/llms/model.ts before provider construction—validate env.BEDROCK_ACCESS_KEY/env.BEDROCK_SECRET_KEY and default providerOptions/providerOptions.openai to empty objects—to prevent runtime errors.

+       if (!env.BEDROCK_ACCESS_KEY || !env.BEDROCK_SECRET_KEY) {
+         throw new Error("Bedrock selected but BEDROCK_ACCESS_KEY and/or BEDROCK_SECRET_KEY are not set");
+       }

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/web/utils/mcp/oauth.ts (1)

493-502: Dynamic registration metadata looks good; consider avoiding hard-coded production URLs

The added logo_uri and tos_uri fields are valid dynamic client registration metadata and safe, and this should work fine with compliant providers.

One concern is maintainability and environment‑awareness: hard‑coding https://getinboxzero.com/... here ties all deployments (staging, dev, self‑hosted) to the production site’s logo and ToS. That may be intentional, but if not, it would be cleaner to derive these from a shared config/env (e.g., a base site URL or branding config) so each environment can advertise its own assets and legal pages.

If that flexibility is desired, I’d suggest something along the lines of:

-  const clientMetadata: OAuthClientMetadata = {
-    client_name: "Inbox Zero",
-    redirect_uris: [redirectUri],
-    grant_types: ["authorization_code", "refresh_token"],
-    response_types: ["code"],
-    token_endpoint_auth_method: "none", // Public client with PKCE
-    scope: integrationConfig.scopes.join(" "),
-    logo_uri: "https://getinboxzero.com/icon.png",
-    tos_uri: "https://getinboxzero.com/terms",
-  };
+  const clientMetadata: OAuthClientMetadata = {
+    client_name: "Inbox Zero",
+    redirect_uris: [redirectUri],
+    grant_types: ["authorization_code", "refresh_token"],
+    response_types: ["code"],
+    token_endpoint_auth_method: "none", // Public client with PKCE
+    scope: integrationConfig.scopes.join(" "),
+    logo_uri: process.env.NEXT_PUBLIC_SITE_LOGO_URL ?? "https://getinboxzero.com/icon.png",
+    tos_uri: process.env.NEXT_PUBLIC_SITE_TOS_URL ?? "https://getinboxzero.com/terms",
+  };

(or equivalent centralized config access used elsewhere in the app).

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d82566e and bc11885.

📒 Files selected for processing (1)
  • apps/web/utils/mcp/oauth.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
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 files

Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

Files:

  • apps/web/utils/mcp/oauth.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)

**/*.{ts,tsx}: For API GET requests to server, use the swr package
Use result?.serverError with toastError from @/components/Toast for 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 convention use[FeatureName]Enabled that return a boolean from useFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming convention use[FeatureName]Variant that define variant types, use useFeatureFlagVariantKey() 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
Use as const instead of literal types and type annotations
Use either T[] or Array<T> consistently
Initialize each enum member value explicitly
Use export type for types
Use `impo...

Files:

  • apps/web/utils/mcp/oauth.ts
**/{server,api,actions,utils}/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)

**/{server,api,actions,utils}/**/*.ts: Use createScopedLogger from "@/utils/logger" for logging in backend code
Add the createScopedLogger instantiation 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, use createScopedLogger().with() to attach context once and reuse the logger without passing variables repeatedly

Files:

  • apps/web/utils/mcp/oauth.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)

Always import Prisma enums from @/generated/prisma/enums instead of @/generated/prisma/client to avoid Next.js bundling errors in client components

Import Prisma using the project's centralized utility: import prisma from '@/utils/prisma'

Files:

  • apps/web/utils/mcp/oauth.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's select option. 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. All findUnique/findFirst calls 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
All findMany queries 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/mcp/oauth.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Use next/image package for images
For API GET requests to server, use the swr package with hooks like useSWR to fetch data
For text inputs, use the Input component with registerProps for form integration and error handling

Files:

  • apps/web/utils/mcp/oauth.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/mcp/oauth.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{js,jsx,ts,tsx}: Don't use accessKey attribute on any HTML element
Don't set aria-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 the scope prop 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 assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex property
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 a title element 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
Assign tabIndex to non-interactive HTML elements with aria-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 a type attribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden)
Always include a lang attribute on the html element
Always include a title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/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/mcp/oauth.ts
!(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:

  • apps/web/utils/mcp/oauth.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/mcp/oauth.ts
⏰ 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). (2)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Review for correctness

@elie222 elie222 merged commit 5a885f2 into staging Dec 4, 2025
14 checks passed
@elie222 elie222 deleted the chore/upgrade-packages branch December 4, 2025 04:53
This was referenced Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant