Skip to content

Comments

Don't force user to log out when requesting new permissions#1069

Merged
elie222 merged 3 commits intomainfrom
fix/permissions
Dec 4, 2025
Merged

Don't force user to log out when requesting new permissions#1069
elie222 merged 3 commits intomainfrom
fix/permissions

Conversation

@elie222
Copy link
Owner

@elie222 elie222 commented Dec 4, 2025

Summary by CodeRabbit

  • New Features
    • Added a client-side "Reconnect account" flow and unified OAuth linking URL resolver for Google/Microsoft.
  • Bug Fixes
    • Consolidated permission-failure routing to the consent page and removed the separate error page.
    • Existing linked accounts now update tokens instead of creating duplicates; callbacks surface token-update results.
  • Tests
    • Updated tests to expect the new token-update return behavior.
  • Chores
    • Version bumped to v2.21.45

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

@vercel
Copy link

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 9:52pm

@coderabbitai
Copy link
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

Consolidates permission and refresh-token checks to a single consent flow, removes the separate permissions error page, adds a client-side reconnect flow using a shared account-linking URL resolver, and introduces an "update_tokens" handling path in OAuth callbacks to refresh tokens for already-linked accounts.

Changes

Cohort / File(s) Summary
Permission check & consent UI
apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx, apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx, apps/web/app/(app)/[emailAccountId]/permissions/error/page.tsx
Merges permission and refresh-token failure checks to always redirect to the consent page; deletes the separate error page; adds client-side reconnect flow on consent page that calls a shared account-linking URL resolver.
Add account UI
apps/web/app/(app)/accounts/AddAccount.tsx
Replaces per-provider fetch flow with getAccountLinkingUrl(provider); renames provider variant from outlook to microsoft; removes manual fetch/response parsing.
Account-linking utility
apps/web/utils/account-linking.ts
Adds `getAccountLinkingUrl(provider: "google"
OAuth account-linking logic & tests
apps/web/utils/oauth/account-linking.ts, apps/web/utils/oauth/account-linking.test.ts
Adds a new return variant { type: "update_tokens"; existingAccountId: string } when the linked account belongs to the same user; tests updated to expect this variant and remove baseUrl usage.
OAuth callback handlers
apps/web/app/api/google/linking/callback/route.ts, apps/web/app/api/outlook/linking/callback/route.ts
Adds handling for linkingResult.type === "update_tokens": persist refreshed tokens (access/refresh/expires_at/scope/token_type) for the existing account, set result to tokens_updated, clear state cookie, and redirect with a success param.
Login form error handling
apps/web/app/(landing)/login/LoginForm.tsx
Wraps provider sign-in calls in try/catch, adds toastError on failures, removes query-param consent branching, and ensures loading flags are reset in finally.
Outlook client error handling
apps/web/utils/outlook/client.ts
Adds additional AADSTS error codes (AADSTS50076, AADSTS50079, AADSTS50158) to the set that trigger re-authentication handling during refresh.
Version bump
version.txt
Bumps version from v2.21.44 to v2.21.45.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Browser
    participant Client as Consent Page (client)
    participant API as /api/{provider}/linking
    participant OAuth as OAuth Provider
    participant Callback as /api/{provider}/linking/callback
    participant DB

    User->>Browser: Click "Reconnect account"
    Browser->>Client: handleReconnect(provider)
    Client->>API: GET /api/{provider}/linking (getAccountLinkingUrl)
    API-->>Client: { url }
    Client->>Browser: window.location = url
    Browser->>OAuth: User authenticates + grants consent
    OAuth->>Callback: Redirect to /api/{provider}/linking/callback?code=...
    Callback->>Callback: handleAccountLinking(...)
    alt existing linked account -> update_tokens
        Callback->>DB: update tokens (access, refresh, expires_at, scope, token_type)
        Callback-->>Browser: 302 -> /accounts?result=tokens_updated
    else new account created
        Callback->>DB: create account and persist tokens
        Callback-->>Browser: 302 -> /accounts?result=success
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Verify correctness of expires_at computation and persisted token fields in both Google and Outlook callback handlers.
  • Confirm all call sites of handleAccountLinking and tests handle the new { type: "update_tokens" } variant.
  • Validate getAccountLinkingUrl request/response shapes and error handling for both providers.
  • Check consent page reconnect UX: loading state, provider mapping, and toast/error flows.

Possibly related PRs

Poem

🐰
I nibble tokens, hop back on the trail,
One reconnect click and no more stale mail.
Consent restored with a gentle thump,
Fresh keys, happy hops — inbox bump! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Don't force user to log out when requesting new permissions' directly summarizes the main change: implementing a reconnection flow that allows users to re-grant permissions without logging out completely.
✨ 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 fix/permissions

📜 Recent 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 769b81f and fca3aab.

📒 Files selected for processing (2)
  • apps/web/utils/oauth/account-linking.test.ts (2 hunks)
  • apps/web/utils/outlook/client.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
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/oauth/account-linking.test.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/oauth/account-linking.test.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/oauth/account-linking.test.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/oauth/account-linking.test.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/oauth/account-linking.test.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/oauth/account-linking.test.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/oauth/account-linking.test.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/oauth/account-linking.test.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/oauth/account-linking.test.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/oauth/account-linking.test.ts
**/*.test.{ts,tsx}

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

**/*.test.{ts,tsx}: Use vitest for testing the application
Tests should be colocated next to the tested file with .test.ts or .test.tsx extension (e.g., dir/format.ts and dir/format.test.ts)
Mock server-only using vi.mock("server-only", () => ({}))
Mock Prisma using vi.mock("@/utils/prisma") and import the mock from @/utils/__mocks__/prisma
Use vi.clearAllMocks() in beforeEach to clean up mocks between tests
Each test should be independent
Use descriptive test names
Mock external dependencies in tests
Do not mock the Logger
Avoid testing implementation details
Use test helpers getEmail, getEmailAccount, and getRule from @/__tests__/helpers for mocking emails, accounts, and rules

Files:

  • apps/web/utils/oauth/account-linking.test.ts
**/*.{test,spec}.{js,jsx,ts,tsx}

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

**/*.{test,spec}.{js,jsx,ts,tsx}: Don't nest describe() blocks too deeply in test files
Don't use callbacks in asynchronous tests and hooks
Don't have duplicate hooks in describe blocks
Don't use export or module.exports in test files
Don't use focused tests
Make sure the assertion function, like expect, is placed inside an it() function call
Don't use disabled tests

Files:

  • apps/web/utils/oauth/account-linking.test.ts
🧠 Learnings (12)
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Use test helpers `getEmail`, `getEmailAccount`, and `getRule` from `@/__tests__/helpers` for mocking emails, accounts, and rules

Applied to files:

  • apps/web/utils/oauth/account-linking.test.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 : Prefer using existing helpers from `@/__tests__/helpers.ts` (`getEmailAccount`, `getEmail`, `getRule`, `getMockMessage`, `getMockExecutedRule`) instead of creating custom test data helpers

Applied to files:

  • apps/web/utils/oauth/account-linking.test.ts
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Mock external dependencies in tests

Applied to files:

  • apps/web/utils/oauth/account-linking.test.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 : Mock 'server-only' module with empty object in LLM test files: `vi.mock("server-only", () => ({}))`

Applied to files:

  • apps/web/utils/oauth/account-linking.test.ts
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Avoid testing implementation details

Applied to files:

  • apps/web/utils/oauth/account-linking.test.ts
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Mock Prisma using `vi.mock("@/utils/prisma")` and import the mock from `@/utils/__mocks__/prisma`

Applied to files:

  • apps/web/utils/oauth/account-linking.test.ts
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Mock `server-only` using `vi.mock("server-only", () => ({}))`

Applied to files:

  • apps/web/utils/oauth/account-linking.test.ts
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Do not mock the Logger

Applied to files:

  • apps/web/utils/oauth/account-linking.test.ts
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Use `vi.clearAllMocks()` in `beforeEach` to clean up mocks between tests

Applied to files:

  • apps/web/utils/oauth/account-linking.test.ts
📚 Learning: 2025-11-25T14:38:08.183Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/logging.mdc:0-0
Timestamp: 2025-11-25T14:38:08.183Z
Learning: Applies to **/{server,api,actions,utils}/**/*.ts : Use `createScopedLogger` from "@/utils/logger" for logging in backend code

Applied to files:

  • apps/web/utils/oauth/account-linking.test.ts
📚 Learning: 2025-11-25T14:38:08.183Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/logging.mdc:0-0
Timestamp: 2025-11-25T14:38:08.183Z
Learning: Applies to **/{server,api,actions,utils}/**/*.ts : Add the `createScopedLogger` instantiation at the top of the file with an appropriate scope name

Applied to files:

  • apps/web/utils/oauth/account-linking.test.ts
📚 Learning: 2025-11-25T14:38:08.183Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/logging.mdc:0-0
Timestamp: 2025-11-25T14:38:08.183Z
Learning: Applies to **/{server,api,actions,utils}/**/*.ts : For large functions with reused variables, use `createScopedLogger().with()` to attach context once and reuse the logger without passing variables repeatedly

Applied to files:

  • apps/web/utils/oauth/account-linking.test.ts
🧬 Code graph analysis (1)
apps/web/utils/oauth/account-linking.test.ts (1)
apps/web/utils/oauth/account-linking.ts (1)
  • handleAccountLinking (17-100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: test
  • GitHub Check: Review for correctness
🔇 Additional comments (4)
apps/web/utils/oauth/account-linking.test.ts (2)

9-13: Env mock for NEXT_PUBLIC_BASE_URL looks good

Mocking @/env with a fixed NEXT_PUBLIC_BASE_URL makes the redirect URL construction deterministic for these tests without leaking into other files. No changes needed.


59-73: update_tokens scenario is correctly exercised

This test cleanly covers the new { type: "update_tokens"; existingAccountId } branch when the provider account is already linked to the same user, and the expectation matches the documented handleAccountLinking contract. The setup and assertion look solid and aligned with the updated behavior.

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

161-176: MFA error codes are valid and correctly implemented.

The three new error codes (AADSTS50076, AADSTS50079, AADSTS50158) have been verified against official Microsoft Entra documentation and are correctly described in the comments:

  • AADSTS50076: MFA required (Conditional Access policy)
  • AADSTS50079: MFA registration required (per-user enforcement or policy)
  • AADSTS50158: External security challenge not satisfied

The implementation appropriately triggers re-authentication for these scenarios and follows the existing error handling pattern.


187-189: The error message is correctly integrated with the new reconnection flow and does not contradict the PR objectives. When this error is triggered, the middleware and client-side error handling properly redirect users to the permissions/consent page, which implements the new account linking infrastructure. No changes are needed.


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.

@macroscopeapp
Copy link
Contributor

macroscopeapp bot commented Dec 4, 2025

Route users missing permissions to in-app consent and add token refresh update flow for Google and Microsoft linking callbacks to avoid forced logout

Unify permission checks to route to /permissions/consent, replace logout-based consent with an in-place OAuth reconnect, and add an update_tokens path in Google and Microsoft linking callbacks to update existing account tokens. Remove the permissions error page and centralize account linking URL retrieval.

📍Where to Start

Start with the permission routing in [PermissionsCheck.tsx](https://github.com/elie222/inbox-zero/pull/1069/files#diff-12cdab0ac00f6ed024ef75cc1e5c8320676a7934eb89ec6b2fb8538c75a1fdb7) and then review the reconnect flow in [permissions/consent/page.tsx](https://github.com/elie222/inbox-zero/pull/1069/files#diff-b31cfb62c2a9361b8358e20fa42da93f388beeb2f8c9e49c8babd94aac5862cc); for backend handling, begin at the Google callback [apps/web/app/api/google/linking/callback/route.ts](https://github.com/elie222/inbox-zero/pull/1069/files#diff-a868226e0163975b542ae0d5ebd02a3c82f87705cd72f6304c36683619e21868) and Microsoft callback [apps/web/app/api/outlook/linking/callback/route.ts](https://github.com/elie222/inbox-zero/pull/1069/files#diff-29d1018c29576cd5df5f9873f847f289093425835637b1ac775bcf465c6d626b) to see the update_tokens branch.


Macroscope summarized fca3aab.

Copy link
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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/utils/oauth/account-linking.ts (1)

25-30: All callers correctly handle the new update_tokens return type; consider using switch statements for exhaustiveness safety

The new { type: "update_tokens"; existingAccountId } branch is already properly handled in both consumers:

  • apps/web/app/api/google/linking/callback/route.ts (lines 208–244): updates tokens for existing account
  • apps/web/app/api/outlook/linking/callback/route.ts (lines 275–311): updates tokens for existing account

Both callers also handle the other three union members (redirect, continue_create, and merge). However, the current pattern uses chained if statements rather than an exhaustive switch, which means TypeScript won't catch missed cases if the return type changes in the future. Consider refactoring to a discriminated union switch statement for stronger type safety and clarity.

🧹 Nitpick comments (4)
apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx (1)

24-31: Unified redirect condition matches new consent flow

The merged condition on hasAllPermissions / hasRefreshToken correctly routes both failure cases to /permissions/consent, which aligns with the new permissions UX and avoids the old logout/error flow.

You might optionally:

  • Handle result?.serverError from checkPermissionsAction (e.g., via toastError) so genuine backend errors don’t silently no-op.
  • Early‑return if !emailAccountId before calling the action, to be extra defensive against transient undefined states.
apps/web/utils/account-linking.ts (1)

1-28: Centralized linking URL helper is solid; consider sharing provider type

The helper cleanly abstracts provider‑specific /api/*/linking/auth-url details and returns a simple url string, which is exactly what the callers need.

Given "google" | "microsoft" now appears in multiple places (here, AddAccount, permissions consent, and OAuth linking), consider extracting a shared type alias (e.g. type EmailProvider = "google" | "microsoft") into a small shared types file to avoid drift and follow the “centralize shared types” guideline.

apps/web/app/api/outlook/linking/callback/route.ts (1)

282-325: Microsoft token‑update flow looks good; expiresAt logic could be shared

The update_tokens branch correctly:

  • Reuses the same expiresAt calculation as the create branch,
  • Updates the relevant token fields,
  • Logs before and after,
  • Stores { success: "tokens_updated" } and redirects with a matching query param while clearing the state cookie.

To avoid future drift, you might extract the expiresAt computation from tokens into a small helper used by both the create and update paths.

apps/web/app/(app)/accounts/AddAccount.tsx (1)

9-31: Add‑account flow correctly reuses getAccountLinkingUrl and new provider union

Switching handleAddAccount to accept "google" | "microsoft" and delegating URL resolution to getAccountLinkingUrl(provider) nicely removes duplicated fetch logic and keeps both buttons wired through a single code path. The Outlook/Microsoft button now correctly calls handleAddAccount("microsoft"), matching the provider string used on the backend.

Given this and other files use the same provider union, centralizing a shared provider type (as noted in the utils comment) would help keep things in sync long term, but the current implementation is functionally sound.

Also applies to: 55-56

📜 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 83f6aa8 and e3ce3c5.

📒 Files selected for processing (10)
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx (1 hunks)
  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx (2 hunks)
  • apps/web/app/(app)/[emailAccountId]/permissions/error/page.tsx (0 hunks)
  • apps/web/app/(app)/accounts/AddAccount.tsx (2 hunks)
  • apps/web/app/(landing)/login/LoginForm.tsx (0 hunks)
  • apps/web/app/api/google/linking/callback/route.ts (1 hunks)
  • apps/web/app/api/outlook/linking/callback/route.ts (1 hunks)
  • apps/web/utils/account-linking.ts (1 hunks)
  • apps/web/utils/oauth/account-linking.ts (2 hunks)
  • version.txt (1 hunks)
💤 Files with no reviewable changes (2)
  • apps/web/app/(landing)/login/LoginForm.tsx
  • apps/web/app/(app)/[emailAccountId]/permissions/error/page.tsx
🧰 Additional context used
📓 Path-based instructions (21)
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/account-linking.ts
  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/utils/oauth/account-linking.ts
  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/api/outlook/linking/callback/route.ts
  • apps/web/app/(app)/accounts/AddAccount.tsx
**/*.{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/account-linking.ts
  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/utils/oauth/account-linking.ts
  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/api/outlook/linking/callback/route.ts
  • apps/web/app/(app)/accounts/AddAccount.tsx
**/{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/account-linking.ts
  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/utils/oauth/account-linking.ts
  • apps/web/app/api/outlook/linking/callback/route.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/account-linking.ts
  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/utils/oauth/account-linking.ts
  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/api/outlook/linking/callback/route.ts
  • apps/web/app/(app)/accounts/AddAccount.tsx
**/*.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/account-linking.ts
  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/utils/oauth/account-linking.ts
  • apps/web/app/api/outlook/linking/callback/route.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/account-linking.ts
  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/utils/oauth/account-linking.ts
  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/api/outlook/linking/callback/route.ts
  • apps/web/app/(app)/accounts/AddAccount.tsx
**/*.{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/account-linking.ts
  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/utils/oauth/account-linking.ts
  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/api/outlook/linking/callback/route.ts
  • apps/web/app/(app)/accounts/AddAccount.tsx
**/*.{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/account-linking.ts
  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/utils/oauth/account-linking.ts
  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/api/outlook/linking/callback/route.ts
  • apps/web/app/(app)/accounts/AddAccount.tsx
!(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/account-linking.ts
  • apps/web/app/api/google/linking/callback/route.ts
  • version.txt
  • apps/web/utils/oauth/account-linking.ts
  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/api/outlook/linking/callback/route.ts
  • apps/web/app/(app)/accounts/AddAccount.tsx
**/*.{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/account-linking.ts
  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/utils/oauth/account-linking.ts
  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/api/outlook/linking/callback/route.ts
  • apps/web/app/(app)/accounts/AddAccount.tsx
apps/web/app/**/*.{ts,tsx}

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

Follow NextJS app router structure with (app) directory

Files:

  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/api/outlook/linking/callback/route.ts
  • apps/web/app/(app)/accounts/AddAccount.tsx
apps/web/app/api/**/*.ts

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

apps/web/app/api/**/*.ts: Wrap GET API routes with withAuth or withEmailAccount middleware for authentication
Export response types from GET API routes using Awaited<ReturnType<>> pattern for type-safe client usage

Files:

  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/app/api/outlook/linking/callback/route.ts
apps/web/app/api/**/route.ts

📄 CodeRabbit inference engine (.cursor/rules/fullstack-workflow.mdc)

apps/web/app/api/**/route.ts: Create GET API routes using withAuth or withEmailAccount middleware in apps/web/app/api/*/route.ts, export response types as GetExampleResponse type alias for client-side type safety
Always export response types from GET routes as Get[Feature]Response using type inference from the data fetching function for type-safe client consumption
Do NOT use POST API routes for mutations - always use server actions with next-safe-action instead

Files:

  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/app/api/outlook/linking/callback/route.ts
**/app/**/route.ts

📄 CodeRabbit inference engine (.cursor/rules/get-api-route.mdc)

**/app/**/route.ts: Always wrap GET API route handlers with withAuth or withEmailAccount middleware for consistent error handling and authentication in Next.js App Router
Infer and export response type for GET API routes using Awaited<ReturnType<typeof functionName>> pattern in Next.js
Use Prisma for database queries in GET API routes
Return responses using NextResponse.json() in GET API routes
Do not use try/catch blocks in GET API route handlers when using withAuth or withEmailAccount middleware, as the middleware handles error handling

Files:

  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/app/api/outlook/linking/callback/route.ts
apps/web/app/**/[!.]*/route.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Use kebab-case for route directories in Next.js App Router (e.g., api/hello-world/route)

Files:

  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/app/api/outlook/linking/callback/route.ts
apps/web/app/api/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/security-audit.mdc)

apps/web/app/api/**/*.{ts,tsx}: API routes must use withAuth, withEmailAccount, or withError middleware for authentication
All database queries must include user scoping with emailAccountId or userId filtering in WHERE clauses
Request parameters must be validated before use; avoid direct parameter usage without type checking
Use generic error messages instead of revealing internal details; throw SafeError instead of exposing user IDs, resource IDs, or system information
API routes should only return necessary fields using select in database queries to prevent unintended information disclosure
Cron endpoints must use hasCronSecret or hasPostCronSecret to validate cron requests and prevent unauthorized access
Request bodies should use Zod schemas for validation to ensure type safety and prevent injection attacks

Files:

  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/app/api/outlook/linking/callback/route.ts
**/app/api/**/*.ts

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

**/app/api/**/*.ts: ALL API routes that handle user data MUST use appropriate middleware: use withEmailAccount for email-scoped operations, use withAuth for user-scoped operations, or use withError with proper validation for public/custom auth endpoints
Use withEmailAccount middleware for operations scoped to a specific email account, including reading/writing emails, rules, schedules, or any operation using emailAccountId
Use withAuth middleware for user-level operations such as user settings, API keys, and referrals that use only userId
Use withError middleware only for public endpoints, custom authentication logic, or cron endpoints. For cron endpoints, MUST use hasCronSecret() or hasPostCronSecret() validation
Cron endpoints without proper authentication can be triggered by anyone. CRITICAL: All cron endpoints MUST validate cron secret using hasCronSecret(request) or hasPostCronSecret(request) and capture unauthorized attempts with captureException()
Always validate request bodies using Zod schemas to ensure type safety and prevent invalid data from reaching database operations
Maintain consistent error response format across all API routes to avoid information disclosure while providing meaningful error feedback

Files:

  • apps/web/app/api/google/linking/callback/route.ts
  • apps/web/app/api/outlook/linking/callback/route.ts
apps/web/**/*.tsx

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

apps/web/**/*.tsx: Follow tailwindcss patterns with prettier-plugin-tailwindcss for class sorting
Prefer functional components with hooks over class components
Use shadcn/ui components when available
Ensure responsive design with mobile-first approach
Use LoadingContent component for async data with loading and error states

Files:

  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/(app)/accounts/AddAccount.tsx
apps/web/app/(app)/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/page-structure.mdc)

apps/web/app/(app)/**/*.{ts,tsx}: Components for the page are either put in page.tsx, or in the apps/web/app/(app)/PAGE_NAME folder
If we're in a deeply nested component we will use swr to fetch via API
If you need to use onClick in a component, that component is a client component and file must start with use client

Files:

  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/(app)/accounts/AddAccount.tsx
**/*.tsx

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

**/*.tsx: Use the LoadingContent component to handle loading states instead of manual loading state management
For text areas, use the Input component with type='text', autosizeTextarea prop set to true, and registerProps for form integration

Files:

  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/(app)/accounts/AddAccount.tsx
**/*.{jsx,tsx}

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

**/*.{jsx,tsx}: Don't use unnecessary fragments
Don't pass children as props
Don't use the return value of React.render
Make sure all dependencies are correctly specified in React hooks
Make sure all React hooks are called from the top level of component functions
Don't forget key props in iterators and collection literals
Don't define React components inside other components
Don't use event handlers on non-interactive elements
Don't assign to React component props
Don't use both children and dangerouslySetInnerHTML props on the same element
Don't use dangerous JSX props
Don't use Array index in keys
Don't insert comments as text nodes
Don't assign JSX properties multiple times
Don't add extra closing tags for components without children
Use <>...</> instead of <Fragment>...</Fragment>
Watch out for possible "wrong" semicolons inside JSX elements
Make sure void (self-closing) elements don't have children
Don't use target="_blank" without rel="noopener"
Don't use <img> elements in Next.js projects
Don't use <head> elements in Next.js projects

Files:

  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/(app)/accounts/AddAccount.tsx
🧠 Learnings (7)
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to apps/web/utils/gmail/**/*.{ts,tsx} : Always use wrapper functions from @/utils/gmail/ for Gmail API operations instead of direct provider API calls

Applied to files:

  • apps/web/utils/account-linking.ts
  • apps/web/app/(app)/accounts/AddAccount.tsx
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to apps/web/utils/gmail/**/*.{ts,tsx} : Keep Gmail provider-specific implementation details isolated within the apps/web/utils/gmail/ directory

Applied to files:

  • apps/web/utils/account-linking.ts
  • apps/web/app/(app)/accounts/AddAccount.tsx
📚 Learning: 2025-07-08T13:14:07.449Z
Learnt from: elie222
Repo: elie222/inbox-zero PR: 537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/(app)/accounts/AddAccount.tsx
📚 Learning: 2025-11-25T14:39:23.326Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:23.326Z
Learning: Applies to app/api/**/*.ts : Use `withEmailAccount` middleware for operations scoped to a specific email account (reading/writing emails, rules, schedules, etc.) - provides `emailAccountId`, `userId`, and `email` in `request.auth`

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
  • apps/web/app/(app)/accounts/AddAccount.tsx
📚 Learning: 2025-11-25T14:39:49.448Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-11-25T14:39:49.448Z
Learning: Applies to apps/web/utils/actions/*.ts : Use `actionClient` when both authenticated user context and a specific emailAccountId are needed, with emailAccountId bound when calling from the client

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/PermissionsCheck.tsx
📚 Learning: 2025-11-25T14:39:27.909Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:27.909Z
Learning: Applies to **/app/api/**/*.ts : Use `withEmailAccount` middleware for operations scoped to a specific email account, including reading/writing emails, rules, schedules, or any operation using `emailAccountId`

Applied to files:

  • apps/web/app/(app)/accounts/AddAccount.tsx
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls

Applied to files:

  • apps/web/app/(app)/accounts/AddAccount.tsx
🧬 Code graph analysis (3)
apps/web/utils/account-linking.ts (2)
apps/web/app/api/google/linking/auth-url/route.ts (1)
  • GetAuthLinkUrlResponse (11-11)
apps/web/app/api/outlook/linking/auth-url/route.ts (1)
  • GetOutlookAuthLinkUrlResponse (10-10)
apps/web/app/api/google/linking/callback/route.ts (3)
apps/web/utils/redis/oauth-code.ts (1)
  • setOAuthCodeResult (43-53)
apps/web/env.ts (1)
  • env (17-246)
apps/web/utils/gmail/constants.ts (1)
  • GOOGLE_LINKING_STATE_COOKIE_NAME (16-16)
apps/web/app/(app)/accounts/AddAccount.tsx (1)
apps/web/utils/account-linking.ts (1)
  • getAccountLinkingUrl (9-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Review for correctness
  • GitHub Check: test
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
version.txt (1)

1-1: Version bump looks good

Version updated to v2.21.45; no behavioral impact and consistent with PR scope.

apps/web/app/api/google/linking/callback/route.ts (1)

211-244: Token‑update branch for Google accounts is correct and symmetric

The update_tokens branch updates exactly the same token fields used on initial account creation, logs before/after, records { success: "tokens_updated" } in the OAuth code result, and redirects with a success=tokens_updated query param while clearing the state cookie. This keeps the flow consistent with the existing create/merge branches and preserves the cached‑result/idempotence behavior above.

Copy link
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.

No issues found across 10 files

Copy link
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: 2

📜 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 e3ce3c5 and 769b81f.

📒 Files selected for processing (2)
  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx (2 hunks)
  • apps/web/app/(landing)/login/LoginForm.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/app/(app)/[emailAccountId]/permissions/consent/page.tsx
🧰 Additional context used
📓 Path-based instructions (13)
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/app/(landing)/login/LoginForm.tsx
apps/web/app/**/*.{ts,tsx}

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

Follow NextJS app router structure with (app) directory

Files:

  • apps/web/app/(landing)/login/LoginForm.tsx
apps/web/**/*.tsx

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

apps/web/**/*.tsx: Follow tailwindcss patterns with prettier-plugin-tailwindcss for class sorting
Prefer functional components with hooks over class components
Use shadcn/ui components when available
Ensure responsive design with mobile-first approach
Use LoadingContent component for async data with loading and error states

Files:

  • apps/web/app/(landing)/login/LoginForm.tsx
**/*.{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/app/(landing)/login/LoginForm.tsx
**/*Form.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)

**/*Form.{ts,tsx}: Use React Hook Form with Zod for validation in form components
Validate form inputs before submission
Show validation errors inline next to form fields

Files:

  • apps/web/app/(landing)/login/LoginForm.tsx
**/*.{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/app/(landing)/login/LoginForm.tsx
**/*.{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/app/(landing)/login/LoginForm.tsx
**/*.{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/app/(landing)/login/LoginForm.tsx
**/*.tsx

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

**/*.tsx: Use the LoadingContent component to handle loading states instead of manual loading state management
For text areas, use the Input component with type='text', autosizeTextarea prop set to true, and registerProps for form integration

Files:

  • apps/web/app/(landing)/login/LoginForm.tsx
**/*.{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/app/(landing)/login/LoginForm.tsx
**/*.{jsx,tsx}

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

**/*.{jsx,tsx}: Don't use unnecessary fragments
Don't pass children as props
Don't use the return value of React.render
Make sure all dependencies are correctly specified in React hooks
Make sure all React hooks are called from the top level of component functions
Don't forget key props in iterators and collection literals
Don't define React components inside other components
Don't use event handlers on non-interactive elements
Don't assign to React component props
Don't use both children and dangerouslySetInnerHTML props on the same element
Don't use dangerous JSX props
Don't use Array index in keys
Don't insert comments as text nodes
Don't assign JSX properties multiple times
Don't add extra closing tags for components without children
Use <>...</> instead of <Fragment>...</Fragment>
Watch out for possible "wrong" semicolons inside JSX elements
Make sure void (self-closing) elements don't have children
Don't use target="_blank" without rel="noopener"
Don't use <img> elements in Next.js projects
Don't use <head> elements in Next.js projects

Files:

  • apps/web/app/(landing)/login/LoginForm.tsx
!(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/app/(landing)/login/LoginForm.tsx
**/*.{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/app/(landing)/login/LoginForm.tsx
🧠 Learnings (10)
📚 Learning: 2025-11-25T14:37:09.306Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/fullstack-workflow.mdc:0-0
Timestamp: 2025-11-25T14:37:09.306Z
Learning: Applies to apps/web/components/**/*Form*.tsx : Handle form submission results using `result?.serverError` to show error toasts and `toastSuccess` to show success messages after server action completion

Applied to files:

  • apps/web/app/(landing)/login/LoginForm.tsx
📚 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/components/**/*.tsx : Use `result?.serverError` with `toastError` and `toastSuccess` for error handling in form submissions

Applied to files:

  • apps/web/app/(landing)/login/LoginForm.tsx
📚 Learning: 2025-11-25T14:36:36.276Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/data-fetching.mdc:0-0
Timestamp: 2025-11-25T14:36:36.276Z
Learning: Applies to **/*.{ts,tsx} : Import error and success toast utilities from '@/components/Toast' for displaying notifications

Applied to files:

  • apps/web/app/(landing)/login/LoginForm.tsx
📚 Learning: 2025-07-08T13:14:07.449Z
Learnt from: elie222
Repo: elie222/inbox-zero PR: 537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.

Applied to files:

  • apps/web/app/(landing)/login/LoginForm.tsx
📚 Learning: 2025-11-25T14:36:40.146Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/data-fetching.mdc:0-0
Timestamp: 2025-11-25T14:36:40.146Z
Learning: Applies to **/*.{ts,tsx} : Use `result?.serverError` with `toastError` from `@/components/Toast` for error handling in async operations

Applied to files:

  • apps/web/app/(landing)/login/LoginForm.tsx
📚 Learning: 2025-11-25T14:36:36.276Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/data-fetching.mdc:0-0
Timestamp: 2025-11-25T14:36:36.276Z
Learning: Applies to **/*.{ts,tsx} : Use `result?.serverError` with `toastError` and `toastSuccess` for error handling in server actions

Applied to files:

  • apps/web/app/(landing)/login/LoginForm.tsx
📚 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/**/*.{ts,tsx} : Use proper error handling with try/catch blocks

Applied to files:

  • apps/web/app/(landing)/login/LoginForm.tsx
📚 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 : Implement early returns for invalid LLM inputs, use proper error types and logging, implement fallbacks for AI failures, and add retry logic for transient failures using `withRetry`

Applied to files:

  • apps/web/app/(landing)/login/LoginForm.tsx
📚 Learning: 2025-11-25T14:37:09.306Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/fullstack-workflow.mdc:0-0
Timestamp: 2025-11-25T14:37:09.306Z
Learning: Applies to apps/web/components/**/*.tsx : Use `LoadingContent` component to consistently handle loading and error states, passing `loading`, `error`, and `children` props

Applied to files:

  • apps/web/app/(landing)/login/LoginForm.tsx
📚 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/**/*.tsx : Use LoadingContent component for async data with loading and error states

Applied to files:

  • apps/web/app/(landing)/login/LoginForm.tsx
🧬 Code graph analysis (1)
apps/web/app/(landing)/login/LoginForm.tsx (1)
apps/web/utils/config.ts (1)
  • WELCOME_PATH (24-24)
⏰ 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: Analyze (javascript-typescript)
  • GitHub Check: test
  • GitHub Check: Review for correctness
🔇 Additional comments (3)
apps/web/app/(landing)/login/LoginForm.tsx (3)

19-19: LGTM!

The toastError import correctly follows the project's error-handling pattern and coding guidelines.


42-43: Excellent improvement to error handling!

The try/catch/finally pattern properly ensures that loading states are reset regardless of success or failure. This addresses the concern raised in the previous review about setLoadingMicrosoft(false) not being called when signIn.social() rejects.

Also applies to: 61-62


31-35: Remove the errorCallbackURL parameter—it's not supported by better-auth and is dead code.

The errorCallbackURL: "/login/error" parameter is not recognized by better-auth's signIn.social() method and serves no function. The only active error handling is the catch block with toastError, which properly handles exceptions. The /login/error route exists but remains unreachable via this parameter. Remove the errorCallbackURL line from both the Google and Microsoft sign-in handlers to avoid confusion about error-handling paths.

⛔ Skipped due to learnings
Learnt from: elie222
Repo: elie222/inbox-zero PR: 537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/get-api-route.mdc:0-0
Timestamp: 2025-11-25T14:37:22.822Z
Learning: Applies to **/app/**/route.ts : Do not use try/catch blocks in GET API route handlers when using `withAuth` or `withEmailAccount` middleware, as the middleware handles error handling
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/get-api-route.mdc:0-0
Timestamp: 2025-11-25T14:37:11.434Z
Learning: Applies to **/app/**/route.ts : Do not use try/catch blocks in GET API route handlers as `withAuth` and `withEmailAccount` middleware handle error handling

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