packages/prisma/migrations/20250819013055_add_calendar_subscription_system/migration.sql (4)

54-55: Consider query patterns: add standalone index on backoffUntil

The composite index (channelExpiration, syncErrors, backoffUntil) won’t help queries that filter primarily on backoffUntil (the leftmost column rule applies). If your scheduler picks “all subs where backoffUntil <= now” regardless of channelExpiration, you’ll get a seq scan.

Add:

+-- CreateIndex
+CREATE INDEX "CalendarSubscription_backoffUntil_idx" ON "CalendarSubscription"("backoffUntil");

---

13-15: Add CHECK constraints to keep error counters sane

Defensive constraints prevent accidental negatives and make invariants explicit.

You can add:

+ALTER TABLE "CalendarSubscription"
+  ADD CONSTRAINT "CalendarSubscription_syncErrors_nonneg_chk" CHECK ("syncErrors" >= 0),
+  ADD CONSTRAINT "CalendarSubscription_maxSyncErrors_nonneg_chk" CHECK ("maxSyncErrors" >= 0);

---

69-70: Reassess global start/end/status index; consider a partial index

A global index on (start, end, status) may be low-selectivity and large. If your common scans target confirmed events, a partial index can reduce bloat and improve performance.

Keep existing indexes for now, but consider adding:

+-- CreateIndex (Postgres partial index example)
+CREATE INDEX "CalendarEvent_start_end_confirmed_idx"
+  ON "CalendarEvent"("start", "end")
+  WHERE "status" = 'confirmed';

You can then measure usage and decide whether to drop "CalendarEvent_start_end_status_idx".

---

40-41: Optional: add recurrence-friendly index

If you query instances by series, an index helps. You already have (calendarSubscriptionId, iCalUID). If queries use recurringEventId or pair iCalUID+originalStartTime, add one accordingly.

For series lookups by recurringEventId:

+-- CreateIndex
+CREATE INDEX "CalendarEvent_calendarSubscriptionId_recurringEventId_idx"
+  ON "CalendarEvent"("calendarSubscriptionId", "recurringEventId");

Also applies to: 62-67

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: The Outlook Calendar integration in Cal.com intentionally reuses subscription IDs across multiple event types for efficiency. The `upsertSelectedCalendarsForEventTypeIds` method creates separate SelectedCalendar records for each eventTypeId, all sharing the same outlookSubscriptionId. This subscription sharing pattern means that unique constraints like `@unique([outlookSubscriptionId, eventTypeId])` should not be applied as they would prevent this intended functionality.

Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like `@unique([outlookSubscriptionId, eventTypeId])` should not be added as they would break this subscription sharing functionality.

packages/features/calendar-cache-sql/CalendarCacheService.ts (5)

92-100: Early return when no valid selected calendar IDs

If selectedCalendars is non-empty but none have an id, selectedCalendarIds becomes empty and the repo call might inadvertently query broadly. Short-circuit to avoid unnecessary/expensive queries.

     const selectedCalendarIds = CalendarCachePresenter.presentCalendarIds(selectedCalendars);
 
+    if (selectedCalendarIds.length === 0) {
+      log.debug("No valid selectedCalendar IDs, returning empty results");
+      return [];
+    }
+
     const subscriptions = await this.subscriptionRepo.findBySelectedCalendarIds(selectedCalendarIds);

---

83-109: Validate and reuse parsed dates to avoid invalid ranges and double parsing

Currently new Date() is called twice and invalid inputs aren’t handled. Validate once, bail early on invalid ranges, and reuse the parsed dates.

   private async loadEventsForSelectedCalendars(
     dateFrom: string,
     dateTo: string,
     selectedCalendars: IntegrationCalendar[]
   ): Promise<CalendarEventForAvailability[]> {
     if (selectedCalendars.length === 0) {
       return [];
     }
 
+    const from = new Date(dateFrom);
+    const to = new Date(dateTo);
+    if (isNaN(from.getTime()) || isNaN(to.getTime())) {
+      log.warn(
+        "Invalid date range provided to loadEventsForSelectedCalendars",
+        safeStringify({ dateFrom, dateTo })
+      );
+      return [];
+    }
+
     const selectedCalendarIds = CalendarCachePresenter.presentCalendarIds(selectedCalendars);
 
     const subscriptions = await this.subscriptionRepo.findBySelectedCalendarIds(selectedCalendarIds);
     const subscriptionIds = CalendarCachePresenter.presentSubscriptionIds(subscriptions);
 
     if (subscriptionIds.length === 0) {
       log.debug("No subscriptions found, returning empty results");
       return [];
     }
 
     const allEvents = await this.eventRepo.getEventsForAvailabilityBatch(
       subscriptionIds,
-      new Date(dateFrom),
-      new Date(dateTo)
+      from,
+      to
     );
 
     return allEvents;
   }

---

118-119: Trim debug logs to avoid logging potentially large objects

Logging full selectedCalendars can be verbose. Log counts and identifiers only.

-    log.debug("Getting availability from cache", safeStringify({ dateFrom, dateTo, selectedCalendars }));
+    log.debug(
+      "Getting availability from cache",
+      safeStringify({ dateFrom, dateTo, selectedCount: selectedCalendars.length })
+    );

-    log.debug(
-      "Getting availability with timezones from cache",
-      safeStringify({ dateFrom, dateTo, selectedCalendars })
-    );
+    log.debug(
+      "Getting availability with timezones from cache",
+      safeStringify({ dateFrom, dateTo, selectedCount: selectedCalendars.length })
+    );

Also applies to: 136-139

---

43-50: Type guard for calendar IDs

If IntegrationCalendar["id"] can be string | undefined | null in some call sites, prefer a type-guard to ensure string[] at compile-time.

-  static presentCalendarIds(selectedCalendars: IntegrationCalendar[]): string[] {
-    return selectedCalendars.reduce<string[]>((acc, sc) => {
-      if (sc.id !== undefined) {
-        acc.push(sc.id);
-      }
-      return acc;
-    }, []);
-  }
+  static presentCalendarIds(selectedCalendars: IntegrationCalendar[]): string[] {
+    return selectedCalendars
+      .map((sc) => sc.id)
+      .filter((id): id is string => typeof id === "string" && id.length > 0);
+  }

Please confirm IntegrationCalendar.id is always string. If it can be number, we should adjust the return type and consumers accordingly.

---

20-20: Minor: extract source string into a constant

Avoid string duplication and typos by centralizing the source value.

-const log = logger.getSubLogger({ prefix: ["CalendarCacheService"] });
+const log = logger.getSubLogger({ prefix: ["CalendarCacheService"] });
+const SOURCE = "calendar-cache-sql";

Then use SOURCE in presenters:

-      source: "calendar-cache-sql",
+      source: SOURCE,

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.

Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.

packages/features/calendar-cache-sql/CalendarSubscriptionRepository.ts (1)

162-171: Type mismatch: channelExpiration should be Date (matches schema/interface)

The interface and Prisma model use DateTime. Using string here risks invalid values and breaks comparisons.

Apply:

   async updateWatchDetails(
     id: string,
     details: {
       channelId: string;
       channelKind?: string;
       channelResourceId?: string;
       channelResourceUri?: string;
-      channelExpiration: string;
+      channelExpiration: Date;
     }
   ) {

packages/features/calendar-cache-sql/CalendarCacheService.ts (2)

31-38: Mask event titles to avoid PII leakage in availability responses

Do not expose event.summary in public/free-busy outputs. Always return “Busy”.

Apply:

   static presentAvailabilityData(events: CalendarEventForAvailability[]): EventBusyDate[] {
     return events.map((event) => ({
       start: event.start.toISOString(),
       end: event.end.toISOString(),
       source: "calendar-cache-sql",
-      title: event.summary || "Busy",
+      title: "Busy",
     }));
   }

---

59-69: Also mask titles in timezone variant

Same rationale as above; avoid leaking summaries.

Apply:

   static presentAvailabilityDataWithTimezones(
     events: CalendarEventForAvailability[]
   ): Ensure<EventBusyDate, "timeZone">[] {
     return events.map((event) => ({
       start: event.start.toISOString(),
       end: event.end.toISOString(),
       timeZone: event.timeZone || "",
       source: "calendar-cache-sql",
-      title: event.summary || "Busy",
+      title: "Busy",
     }));
   }

packages/features/calendar-cache-sql/CalendarSubscriptionRepository.ts (3)

61-69: Use explicit select to comply with Prisma “select-only” guideline

This read currently returns all columns. Add a select projection consistent with other methods to keep the contract tight.

Apply:

   async findByCredentialId(credentialId: number) {
     return await this.prismaClient.calendarSubscription.findFirst({
       where: {
         selectedCalendar: {
           credentialId,
         },
       },
+      select: {
+        id: true,
+        selectedCalendarId: true,
+        channelId: true,
+        channelKind: true,
+        channelResourceId: true,
+        channelResourceUri: true,
+        channelResource: true,
+        clientState: true,
+        channelExpiration: true,
+        syncCursor: true,
+        syncErrors: true,
+        maxSyncErrors: true,
+        backoffUntil: true,
+        createdAt: true,
+        updatedAt: true,
+      },
     });
   }

---

136-150: Batch upsert: consider chunking to avoid large concurrent bursts

Using Promise.all is fine for small/medium sets. If this ever handles hundreds/thousands of IDs, chunk requests to keep DB and connection pool healthy.

I can provide a chunking helper if you expect large batches.

---

241-265: Optional: Use the error parameter for observability and progressive backoff context

The _error arg is ignored. Consider logging it at debug/warn level to aid troubleshooting, without storing sensitive payloads.

-  async setWatchError(id: string, _error: string) {
+  async setWatchError(id: string, error: string) {
     await this.prismaClient.$transaction(async (tx) => {
+      // Optionally log error context (avoid logging tokens/PII)
+      // log.warn({ id, error }, "Set watch error");

packages/features/calendar-cache-sql/CalendarCacheService.ts (1)

118-121: Reduce PII in logs: avoid logging full selectedCalendars payload

Logging the entire selectedCalendars can include emails and other PII. Log counts and IDs instead.

Apply:

-    log.debug("Getting availability from cache", safeStringify({ dateFrom, dateTo, selectedCalendars }));
+    const selectedCalendarIds = CalendarCachePresenter.presentCalendarIds(selectedCalendars);
+    log.debug(
+      "Getting availability from cache",
+      safeStringify({ dateFrom, dateTo, selectedCalendarCount: selectedCalendars.length, selectedCalendarIds })
+    );

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.

Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.

Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.

Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).

Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : For Prisma queries, only select data you need; never use `include`, always use `select`

Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: The Outlook Calendar integration in Cal.com intentionally reuses subscription IDs across multiple event types for efficiency. The `upsertSelectedCalendarsForEventTypeIds` method creates separate SelectedCalendar records for each eventTypeId, all sharing the same outlookSubscriptionId. This subscription sharing pattern means that unique constraints like `@unique([outlookSubscriptionId, eventTypeId])` should not be applied as they would prevent this intended functionality.

Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/lib/server/repository/PrismaPhoneNumberRepository.ts:412-417
Timestamp: 2025-08-07T18:42:34.081Z
Learning: In Cal.com codebase, the coding guideline requiring explicit `select` clauses instead of `include` for Prisma queries applies to read operations but not to update operations. Update operations don't need explicit select clauses.

packages/features/calendar-cache-sql/CalendarSubscriptionRepository.ts (1)

181-239: Fix invalid field-to-field comparison in Prisma filter; filter in application layer and add deterministic ordering.

this.prismaClient.calendarSubscription.fields.maxSyncErrors is not a valid Prisma construct. Prisma does not support column-to-column comparisons in where. Fetch a superset, filter syncErrors < maxSyncErrors in memory, then slice to the limit. Also add an orderBy for determinism.

Apply:

   async getSubscriptionsToWatch(limit = 100) {
     const oneDayInMS = 24 * 60 * 60 * 1000;
     const tomorrow = new Date(Date.now() + oneDayInMS);

-    return await this.prismaClient.calendarSubscription.findMany({
-      take: limit,
+    const rows = await this.prismaClient.calendarSubscription.findMany({
+      // Over-fetch to compensate for post-filtering (syncErrors < maxSyncErrors)
+      take: limit * 3,
       where: {
         selectedCalendar: {
           user: {
             teams: {
               some: {
                 team: {
                   features: {
                     some: {
                       featureId: CALENDAR_CACHE_SQL_WRITE_FEATURE,
                     },
                   },
                 },
               },
             },
           },
           integration: "google_calendar",
         },
-        syncErrors: {
-          lt: this.prismaClient.calendarSubscription.fields.maxSyncErrors,
-        },
         OR: [{ channelExpiration: null }, { channelExpiration: { lt: tomorrow } }],
         AND: [{ OR: [{ backoffUntil: null }, { backoffUntil: { lte: new Date() } }] }],
       },
+      orderBy: [{ channelExpiration: "asc" }, { updatedAt: "asc" }],
       select: {
         id: true,
         selectedCalendarId: true,
         channelId: true,
         channelKind: true,
         channelResourceId: true,
         channelResourceUri: true,
         channelResource: true,
         clientState: true,
         channelExpiration: true,
         syncCursor: true,
         syncErrors: true,
         maxSyncErrors: true,
         backoffUntil: true,
         createdAt: true,
         updatedAt: true,
         selectedCalendar: {
           select: {
             id: true,
             externalId: true,
             integration: true,
             userId: true,
             credential: {
               select: safeCredentialSelectForCalendarCache,
             },
           },
         },
       },
     });
+    return rows.filter((s) => s.syncErrors < s.maxSyncErrors).slice(0, limit);
   }

packages/features/calendar-cache-sql/CalendarSubscriptionRepository.ts (5)

71-84: Early-return on empty array to skip unnecessary DB call.

Minor optimization: if selectedCalendarIds is empty, return [] immediately.

Apply:

 async findBySelectedCalendarIds(selectedCalendarIds: string[]) {
-  return await this.prismaClient.calendarSubscription.findMany({
+  if (selectedCalendarIds.length === 0) return [];
+  return await this.prismaClient.calendarSubscription.findMany({
     where: {
       selectedCalendarId: {
         in: selectedCalendarIds,
       },
     },
     select: {
       id: true,
       selectedCalendarId: true,
       updatedAt: true,
     },
   });
 }

---

136-150: Avoid unbounded Promise.all inside a transaction; chunk for scalability.

Large arrays can create many concurrent operations within a single transaction. Chunking reduces DB pressure and memory spikes.

Apply:

 async upsertManyBySelectedCalendarId(selectedCalendarIds: string[]) {
   // Use transactional callback for better compatibility with Prismock tests
   return await this.prismaClient.$transaction(async (tx) => {
-    const results = await Promise.all(
-      selectedCalendarIds.map((selectedCalendarId) =>
-        tx.calendarSubscription.upsert({
-          where: { selectedCalendarId },
-          create: { selectedCalendar: { connect: { id: selectedCalendarId } } },
-          update: { updatedAt: new Date() },
-        })
-      )
-    );
-    return results;
+    const results: Prisma.CalendarSubscription[] = [];
+    const BATCH_SIZE = 50;
+    for (let i = 0; i < selectedCalendarIds.length; i += BATCH_SIZE) {
+      const slice = selectedCalendarIds.slice(i, i + BATCH_SIZE);
+      const batch = await Promise.all(
+        slice.map((selectedCalendarId) =>
+          tx.calendarSubscription.upsert({
+            where: { selectedCalendarId },
+            create: { selectedCalendar: { connect: { id: selectedCalendarId } } },
+            update: { updatedAt: new Date() },
+          })
+        )
+      );
+      results.push(...batch);
+    }
+    return results;
   });
 }

---

152-160: Nit: Method name vs field name mismatch (token vs cursor).

updateSyncToken writes syncCursor. Consider renaming to updateSyncCursor (or documenting the mapping) to avoid confusion.

---

241-265: setWatchError: Progressive backoff is good; consider using the error param or documenting why it’s ignored.

If the schema has a place to store the last error (message/timestamp), consider persisting it. Otherwise, add a short comment indicating the error string is handled upstream to avoid confusion.

Where is the error parameter consumed or logged today (service layer? monitoring)? If not used, should we remove it from the interface or store it for troubleshooting?

---

61-69: Consider narrowing selected fields since only existence is checked

In the only call site (getCalendar.ts), the returned subscription is used solely to test for existence (truthiness), not to access any of its properties. You can reduce query payload and improve clarity by:

• Adding a select: { id: true } (or minimal required fields) to findByCredentialId
• Alternatively, renaming it to something like existsByCredentialId and returning a boolean

If you ever need full subscription data downstream, update this method’s select accordingly.

/// packages/features/calendar-cache-sql/CalendarSubscriptionRepository.ts
 async findByCredentialId(credentialId: number) {
-  return await this.prismaClient.calendarSubscription.findFirst({
-    where: { selectedCalendar: { credentialId } },
-  });
+  return await this.prismaClient.calendarSubscription.findFirst({
+    where: { selectedCalendar: { credentialId } },
+    select: { id: true }, // only fetch the primary key since we only check existence
+  });
 }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : For Prisma queries, only select data you need; never use `include`, always use `select`

Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: The Outlook Calendar integration in Cal.com intentionally reuses subscription IDs across multiple event types for efficiency. The `upsertSelectedCalendarsForEventTypeIds` method creates separate SelectedCalendar records for each eventTypeId, all sharing the same outlookSubscriptionId. This subscription sharing pattern means that unique constraints like `@unique([outlookSubscriptionId, eventTypeId])` should not be applied as they would prevent this intended functionality.

Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/lib/server/repository/PrismaPhoneNumberRepository.ts:412-417
Timestamp: 2025-08-07T18:42:34.081Z
Learning: In Cal.com codebase, the coding guideline requiring explicit `select` clauses instead of `include` for Prisma queries applies to read operations but not to update operations. Update operations don't need explicit select clauses.