Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"analyze:browser": "BUNDLE_ANALYZE=browser next build",
"clean": "rm -rf .turbo && rm -rf node_modules && rm -rf .next",
"dev": "yarn copy-static && next dev --turbopack",
"dev:cron": "ts-node cron-tester.ts",
"dev:cron": "npx tsx cron-tester.ts",
"dev-https": "NODE_TLS_REJECT_UNAUTHORIZED=0 next dev --experimental-https",
"dx": "yarn dev",
"test-codegen": "yarn playwright codegen http://localhost:3000",
Expand Down
7 changes: 7 additions & 0 deletions apps/web/public/static/locales/en/common.json
Original file line number Diff line number Diff line change
Expand Up @@ -3379,5 +3379,12 @@
"timezone_mismatch_tooltip": "You are viewing the report based on your profile timezone ({{userTimezone}}), while your browser is set to timezone ({{browserTimezone}})",
"failed_bookings_by_field": "Failed Bookings By Field",
"event_type_no_hosts": "No hosts are assigned to event type",
"cache_status": "Cache Status",
"cache_last_updated": "Last updated: {{timestamp}}",
"delete_cached_data": "Delete cached data",
"cache_deleted_successfully": "Cache deleted successfully",
"error_deleting_cache": "Error deleting cache",
"confirm_delete_cache": "Are you sure you want to delete the cached data? This action cannot be undone.",
"yes_delete_cache": "Yes, delete cache",
"ADD_NEW_STRINGS_ABOVE_THIS_LINE_TO_PREVENT_MERGE_CONFLICTS": "↑↑↑↑↑↑↑↑↑↑↑↑↑ Add your new strings above here ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑"
}
3 changes: 3 additions & 0 deletions packages/app-store/googlecalendar/lib/CalendarService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,9 @@ export default class GoogleCalendarService implements Calendar {
const data = await this.fetchAvailability(parsedArgs);
await this.setAvailabilityInCache(parsedArgs, data);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 CRITICAL — Intent Alignment (confidence: 100%)

The intent states 'When Google Calendar fetches availability and updates the cache, it also calls SelectedCalendarRepository.updateManyByCredentialId to update updatedAt for all selected calendars under that credential.' However, the cacheUpdatedAt displayed in the UI comes from CalendarCache.updatedAt (queried via CalendarCacheRepository.getCacheStatusByCredentialIds), not from SelectedCalendar.updatedAt. Updating SelectedCalendar.updatedAt does not affect the cache status displayed in the dropdown. The CalendarCache.updatedAt field has @updatedAt in the Prisma schema, so it should auto-update when cache rows are modified via setAvailabilityInCache. This SelectedCalendarRepository.updateManyByCredentialId call appears to update the wrong table for its stated purpose.

Evidence:

  • connectedCalendars.handler.ts queries CalendarCacheRepository.getCacheStatusByCredentialIds which reads CalendarCache.updatedAt
  • CalendarService.ts calls SelectedCalendarRepository.updateManyByCredentialId which updates SelectedCalendar table
  • These are two different tables - SelectedCalendar vs CalendarCache
  • The CalendarCache model already has updatedAt DateTime @default(now()) @updatedAt which auto-updates on Prisma writes
  • The setAvailabilityInCache call on line 1020 already writes to CalendarCache, which should trigger the @updatedAt auto-update on CalendarCache

Agent: logic

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 CRITICAL — Business Logic Correctness (confidence: 98%)

Calling updateManyByCredentialId with an empty object {} as the data payload may not trigger Prisma's @updatedAt auto-update. Prisma's @updatedAt directive updates the timestamp when a record is modified via Prisma client, but updateMany with an empty data object may be treated as a no-op or may not trigger the @updatedAt behavior since no actual fields are being changed. This means SelectedCalendar.updatedAt may never get updated, silently breaking the cache timestamp tracking feature.

Evidence:

  • The intent states 'reviewers should verify that passing {} as the update data actually updates the updatedAt field via Prisma's auto-update behavior'
  • Prisma's @updatedAt is documented to update the field 'automatically stores the time when a record was last updated' - but this requires an actual update operation with field changes
  • The updateMany Prisma method with empty data {} may generate a SQL UPDATE with no SET clauses or skip the query entirely
  • This is also flagged as a risk area: 'Prisma's @updatedAt auto-update may or may not trigger on empty update payloads; if it doesn't, the cache timestamp tracking will silently fail'
  • Note: The intent says to update SelectedCalendar.updatedAt but the feature tracks CalendarCache.updatedAt - however the code is calling SelectedCalendarRepository, not CalendarCacheRepository

Agent: logic

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 CRITICAL — Business Logic Correctness (confidence: 100%)

Calling updateManyByCredentialId(credentialId, {}) with an empty data object will NOT trigger Prisma's @updatedAt auto-update on SelectedCalendar. Prisma's @updatedAt directive only sets the timestamp when actual data fields are being written in an update operation. Passing an empty {} as the data argument means no fields are being modified, so Prisma either treats it as a no-op or does not generate the updatedAt assignment in the SQL. This means the cache status timestamp shown to users will never actually get updated after a fetch cycle, defeating the purpose of the feature.

Evidence:

  • The intent states: 'After a cache fetch cycle completes in GoogleCalendarService.fetchAvailabilityAndSetCache, SelectedCalendar.updatedAt is updated for all calendars under that credential via SelectedCalendarRepository.updateManyByCredentialId'
  • The risk areas explicitly call this out: 'passing an empty data object may not correctly trigger Prisma's @updatedat auto-update on SelectedCalendar, making the cache status update a no-op'
  • Prisma's updateMany with data: {} generates no SET clauses, and @updatedAt is only injected when Prisma detects an actual update operation with fields to write
  • Furthermore, the feature reads cacheUpdatedAt from CalendarCache.updatedAt (via getCacheStatusByCredentialIds), NOT from SelectedCalendar.updatedAt, so this entire call may be targeting the wrong model for the displayed timestamp

Agent: logic

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Data Integrity / Silent Failure (confidence: 100%)

Calling SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {}) with an empty data object {} will not reliably trigger Prisma's @updatedAt auto-update. Prisma only updates @updatedAt when at least one actual field is included in the data payload. If SelectedCalendar.updatedAt does not have @updatedAt in the schema, this call is a no-op and the cache status timestamp will never be updated — silently breaking the feature's core purpose of tracking when the cache was last refreshed.

Evidence:

  • Prisma updateMany with data: {} behavior: Prisma skips the update if no fields are provided, meaning @updatedAt is not set
  • The PR intent is to use this call to 'touch' the updatedAt field, but {} does not constitute a field update
  • If SelectedCalendar lacks @updatedAt on its updatedAt field, the call is functionally a no-op
  • The review context's edge case section explicitly flags: 'updateManyByCredentialId(credentialId, {}) with an empty data object — Prisma updateMany with empty data may be a no-op'

Agent: security

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 CRITICAL — Business logic correctness (confidence: 100%)

Calling updateManyByCredentialId(credentialId, {}) with an empty data object will likely NOT trigger Prisma's @updatedAt directive. Prisma's updateMany with an empty data object is effectively a no-op — no SQL SET clause is generated, so the @updatedAt auto-update mechanism never fires. This means SelectedCalendar.updatedAt will never be refreshed after cache warmup, defeating the purpose of tracking when the cache was last updated.

Evidence:

  • The intent states: 'When GoogleCalendarService.warmupCache completes, SelectedCalendar.updatedAt should be updated for all calendars under that credential'
  • The edge cases explicitly call this out: 'Prisma's updateMany with empty data may be a no-op depending on the version — the @updatedat field may not auto-update if no fields are explicitly changed'
  • The repository method signature accepts Prisma.SelectedCalendarUpdateInput and passes it directly as data: {} to updateMany
  • Prisma documentation states @updatedat is only set when a write operation modifies data

Agent: logic

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Logic Bug / Silent Failure (confidence: 98%)

Calling SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {}) with an empty data object {} to trigger Prisma's @updatedAt auto-update is unreliable. Prisma's updateMany with an empty data object may be a no-op at the database level — the @updatedAt field is only auto-populated when at least one other field is updated. If Prisma optimizes away the empty update, the updatedAt timestamp will never be written, silently breaking the cache freshness UI feature.

Evidence:

  • Line 1022: await SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {});
  • Prisma documentation states @updatedAt is set automatically on update/updateMany operations, but behavior with empty data: {} is not explicitly guaranteed
  • In practice, many Prisma versions will emit a SQL UPDATE with only updatedAt = NOW() when @updatedAt is present, but this depends on version and may silently skip the query
  • The entire cacheUpdatedAt feature in the UI depends on this mechanism working correctly

Agent: security

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))

Calling updateManyByCredentialId(credentialId, {}) with an empty data object will likely NOT trigger Prisma's @updatedAt directive. Prisma's updateMany with an empty data object is effectively a no-op — no SQL SET clause is generated, so the @updatedAt auto-update mechanism never fires. This means SelectedCalendar.updatedAt will never be refreshed after cache warmup, defeating the purpose of tracking when the cache was last updated.

Suggested change
await SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, { updatedAt: new Date() });

🤖 Grapple PR auto-fix • critical • confidence: 100%

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Grapple PR] Auto-fix — security agent (Small fix (2 lines, 1 file))

Calling SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {}) with an empty data object {} to trigger Prisma's @updatedAt auto-update is unreliable. Prisma's updateMany with an empty data object may be a no-op at the database level — the @updatedAt field is only auto-populated when at least one other field is updated. If Prisma optimizes away the empty update, the updatedAt timestamp will never be written, silently breaking the cache freshness UI feature.

Suggested change
await SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, { updatedAt: new Date() });

🤖 Grapple PR auto-fix • minor • confidence: 98%

// Update SelectedCalendar.updatedAt for all calendars under this credential

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Scalability concerns (confidence: 97%)

Passing an empty object {} as the data argument to updateManyByCredentialId relies on Prisma's @updatedAt directive auto-updating the timestamp. However, Prisma's updateMany with an empty data object may generate a no-op SQL statement (e.g., UPDATE ... SET WHERE ... with no SET clauses) or throw an error, because @updatedAt is a Prisma-level directive that only triggers when actual data fields are provided in the update. This means the updatedAt field on SelectedCalendar may never actually get updated, silently breaking the cache timestamp tracking feature.

Evidence:

  • Line 1023: await SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {});
  • Prisma docs state @updatedAt automatically stores the time when a record is last updated - but updateMany with empty data may not constitute an 'update'
  • The updateManyByCredentialId method passes data directly to prisma.selectedCalendar.updateMany({ where: { credentialId }, data })
  • If Prisma optimizes away empty updates, no SQL UPDATE is issued and @updatedAt won't trigger

Agent: architecture

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Behavioral Correctness (confidence: 100%)

Calling SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {}) with an empty data object {} is intended to trigger Prisma's @updatedAt auto-update on the SelectedCalendar model. However, Prisma's updateMany with an empty data object may execute as a no-op (no SET clause) or throw, depending on the Prisma version. Additionally, this relies on SelectedCalendar.updatedAt having the @updatedAt Prisma attribute — if it's just a regular DateTime column without @updatedAt, the auto-update won't fire. This needs verification against the Prisma schema to ensure the intended behavior actually occurs.

Evidence:

  • The comment says 'Update SelectedCalendar.updatedAt for all calendars under this credential'
  • The data argument is {} — an empty object
  • Prisma's @updatedAt only auto-sets when an actual update is performed; an empty data object may not constitute an update
  • The new CalendarCache.updatedAt field has @updatedAt, but the SelectedCalendar model's updatedAt attribute is not shown in this diff — if it lacks @updatedAt, this is definitely a no-op

Agent: architecture

await SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {});
}

async createSelectedCalendar(
Expand Down
157 changes: 157 additions & 0 deletions packages/features/apps/components/CredentialActionsDropdown.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
"use client";

import { useState } from "react";

import { useLocale } from "@calcom/lib/hooks/useLocale";
import { GOOGLE_CALENDAR_TYPE } from "@calcom/platform-constants";
import { trpc } from "@calcom/trpc/react";
import { Button } from "@calcom/ui/components/button";
import { ConfirmationDialogContent } from "@calcom/ui/components/dialog";
import { Dialog } from "@calcom/ui/components/dialog";
import {
Dropdown,
DropdownItem,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuTrigger,
} from "@calcom/ui/components/dropdown";
import { showToast } from "@calcom/ui/components/toast";

interface CredentialActionsDropdownProps {
credentialId: number;
integrationType: string;
cacheUpdatedAt?: Date | null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Data flow (confidence: 88%)

The cacheUpdatedAt prop is typed as Date | null but the data coming from the tRPC response will be a JSON-serialized string (ISO format), not a Date object. The code at line 96 does new Date(cacheUpdatedAt) which handles this correctly at runtime, but the type annotation is misleading and could cause issues if someone relies on it being a Date object elsewhere.

Evidence:

  • tRPC serializes Date objects to ISO strings over the wire
  • Line 96: new Date(cacheUpdatedAt) — this works with both strings and Dates, showing the developer was aware of the potential string input
  • The interface declares cacheUpdatedAt?: Date | null but the actual runtime value from connectedCalendarsHandler will be a string after JSON serialization

Agent: logic

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))

The cacheUpdatedAt prop is typed as Date | null but the data coming from the tRPC response will be a JSON-serialized string (ISO format), not a Date object. The code at line 96 does new Date(cacheUpdatedAt) which handles this correctly at runtime, but the type annotation is misleading and could cause issues if someone relies on it being a Date object elsewhere.

Suggested change
cacheUpdatedAt?: Date | null;
cacheUpdatedAt?: string | null;

🤖 Grapple PR auto-fix • minor • confidence: 88%

onSuccess?: () => void;
delegationCredentialId?: string | null;
disableConnectionModification?: boolean;
}

export default function CredentialActionsDropdown({
credentialId,
integrationType,
cacheUpdatedAt,
onSuccess,
delegationCredentialId,
disableConnectionModification,
}: CredentialActionsDropdownProps) {
const { t } = useLocale();
const [dropdownOpen, setDropdownOpen] = useState(false);
const [deleteModalOpen, setDeleteModalOpen] = useState(false);
const [disconnectModalOpen, setDisconnectModalOpen] = useState(false);

const deleteCacheMutation = trpc.viewer.calendars.deleteCache.useMutation({
onSuccess: () => {
showToast(t("cache_deleted_successfully"), "success");
onSuccess?.();
},
onError: () => {
showToast(t("error_deleting_cache"), "error");
},
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Cross-service impact (confidence: 100%)

After deleteCacheMutation succeeds, it calls onSuccess?.() but does not invalidate connectedCalendars query cache. The cacheUpdatedAt value displayed in the dropdown will remain stale until the parent independently re-fetches. In contrast, disconnectMutation properly invalidates both connectedCalendars and integrations queries in its onSettled callback.

Evidence:

  • Lines 46-50: deleteCacheMutation only calls onSuccess?.() and shows toast on success
  • Lines 55-63: disconnectMutation has onSettled that invalidates utils.viewer.calendars.connectedCalendars
  • After cache deletion, the dropdown would still show the old cacheUpdatedAt timestamp until a manual refetch occurs

Agent: architecture


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Data Freshness / State Management (confidence: 100%)

After successfully deleting the cache via deleteCacheMutation, the component only calls onSuccess?.() but does not invalidate the connectedCalendars query. The dropdown will continue showing the old cacheUpdatedAt timestamp until the parent component re-fetches data. In contrast, the disconnectMutation properly invalidates connectedCalendars in its onSettled handler.

Evidence:

  • Lines 44-51: deleteCacheMutation only calls onSuccess?.() on success
  • Lines 53-64: disconnectMutation has onSettled that calls utils.viewer.calendars.connectedCalendars.invalidate()
  • After cache deletion, cacheUpdatedAt should become null, but the dropdown will still show the old timestamp
  • The intent notes: 'after cache deletion, the cacheUpdatedAt displayed in the dropdown will not refresh until the parent component re-fetches'

Agent: logic

const utils = trpc.useUtils();
const disconnectMutation = trpc.viewer.credentials.delete.useMutation({
onSuccess: () => {
showToast(t("app_removed_successfully"), "success");
onSuccess?.();
},
onError: () => {
showToast(t("error_removing_app"), "error");
},
async onSettled() {
await utils.viewer.calendars.connectedCalendars.invalidate();
await utils.viewer.apps.integrations.invalidate();
},
});

const isGoogleCalendar = integrationType === GOOGLE_CALENDAR_TYPE;
const canDisconnect = !delegationCredentialId && !disableConnectionModification;
const hasCache = isGoogleCalendar && cacheUpdatedAt;

if (!canDisconnect && !hasCache) {
return null;
}

return (
<>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 INFO — Code patterns (confidence: 79%)

The cacheStatusMap.get() uses || null which will coerce falsy values (including timestamp 0 for epoch) to null. While unlikely for timestamps, this could mask legitimate cache data.

Evidence:

  • Line 74: cacheStatusMap.get(calendar.credentialId) || null
  • The || null pattern coerces any falsy value to null
  • Edge case: if updatedAt is somehow 0 (epoch), it would be treated as null
  • Better to explicitly check for undefined

Agent: style

<Dropdown open={dropdownOpen} onOpenChange={setDropdownOpen}>
<DropdownMenuTrigger asChild>
<Button type="button" variant="icon" color="secondary" StartIcon="ellipsis" />
</DropdownMenuTrigger>
<DropdownMenuContent>
{hasCache && (
<>
<DropdownMenuItem className="focus:ring-muted">
<div className="px-2 py-1">
<div className="text-sm font-medium text-gray-900 dark:text-white">{t("cache_status")}</div>
<div className="text-xs text-gray-500 dark:text-white">
{t("cache_last_updated", {
timestamp: new Intl.DateTimeFormat("en-US", {
dateStyle: "short",
timeStyle: "short",
}).format(new Date(cacheUpdatedAt)),
interpolation: { escapeValue: false },
})}
</div>
</div>
</DropdownMenuItem>
<DropdownMenuItem className="outline-none">
<DropdownItem

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Timestamp Formatting / i18n (confidence: 95%)

The timestamp is formatted using a hardcoded en-US locale in Intl.DateTimeFormat instead of using the user's locale. This means non-English users will always see US-formatted dates (e.g., '7/15/25, 3:30 PM') regardless of their language settings.

Evidence:

  • Line 95: new Intl.DateTimeFormat('en-US', { dateStyle: 'short', timeStyle: 'short' })
  • The component already imports and uses useLocale for translations
  • The i18n key cache_last_updated accepts a timestamp interpolation, but the timestamp itself is hardcoded to en-US format

Agent: logic

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Pattern violation (confidence: 93%)

The timestamp formatting is hardcoded to en-US locale with a new Intl.DateTimeFormat("en-US", ...). This bypasses the user's locale preference. For an i18n-aware application that already uses useLocale(), the date format should respect the user's language/locale setting.

Evidence:

  • Line 93: new Intl.DateTimeFormat("en-US", { dateStyle: "short", timeStyle: "short" })
  • The component already imports and uses useLocale for translations
  • Cal.com supports multiple locales, and date formatting should be consistent with the user's preference

Agent: architecture

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Data Flow (confidence: 94%)

The timestamp formatting is hardcoded to en-US locale instead of using the user's locale. This is inconsistent with the i18n approach used elsewhere in the component (using useLocale and t() for translations). Users in non-English locales will see a date format that doesn't match their expected locale conventions.

Evidence:

  • The component imports and uses useLocale for all text strings but hardcodes 'en-US' for the date formatter
  • Cal.com is an internationalized app; date formatting should respect the user's locale

Agent: logic

type="button"
color="destructive"
StartIcon="trash"
onClick={() => {
setDeleteModalOpen(true);
setDropdownOpen(false);
}}>
{t("delete_cached_data")}
</DropdownItem>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Data Exposure (confidence: 93%)

The timestamp is formatted with a hardcoded 'en-US' locale instead of using the user's locale. While not a direct security vulnerability, this exposes implementation assumptions and could leak unintended information about server-side locale configuration. More importantly, the i18n interpolation passes interpolation: { escapeValue: false } which disables XSS escaping for the timestamp value. Although Intl.DateTimeFormat output is safe here, this pattern is dangerous if copy-pasted with user-controlled input.

Evidence:

  • Line 101: new Intl.DateTimeFormat('en-US', ...) — hardcoded locale ignores user preferences
  • Line 106: interpolation: { escapeValue: false } — disables i18next's XSS escaping for this translation
  • The timestamp value comes from Intl.DateTimeFormat here, which is safe, but the pattern of disabling escaping is a footgun for future changes
  • PR acceptance criteria: 'reviewers should verify the actual timestamp formatting in the component'

Agent: security

</DropdownMenuItem>
</>
)}
{canDisconnect && hasCache && <hr className="my-1" />}
{canDisconnect && (
<DropdownMenuItem className="outline-none">
<DropdownItem
type="button"
color="destructive"
StartIcon="trash"
onClick={() => {
setDisconnectModalOpen(true);
setDropdownOpen(false);
}}>
{t("remove_app")}
</DropdownItem>
</DropdownMenuItem>
)}
</DropdownMenuContent>
</Dropdown>

<Dialog open={deleteModalOpen} onOpenChange={setDeleteModalOpen}>
<ConfirmationDialogContent
variety="danger"
title={t("delete_cached_data")}
confirmBtnText={t("yes_delete_cache")}
onConfirm={() => {
deleteCacheMutation.mutate({ credentialId });
setDeleteModalOpen(false);
}}>
{t("confirm_delete_cache")}
</ConfirmationDialogContent>
</Dialog>

<Dialog open={disconnectModalOpen} onOpenChange={setDisconnectModalOpen}>
<ConfirmationDialogContent
variety="danger"
title={t("remove_app")}
confirmBtnText={t("yes_remove_app")}
onConfirm={() => {
disconnectMutation.mutate({ id: credentialId });
setDisconnectModalOpen(false);
}}>
{t("are_you_sure_you_want_to_remove_this_app")}
</ConfirmationDialogContent>
</Dialog>
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,7 @@ export interface ICalendarCacheRepository {
userId: number | null;
args: FreeBusyArgs;
}): Promise<CalendarCache | null>;
getCacheStatusByCredentialIds(
credentialIds: number[]
): Promise<{ credentialId: number; updatedAt: Date | null }[]>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,9 @@ export class CalendarCacheRepositoryMock implements ICalendarCacheRepository {
async deleteManyByCredential() {
log.info(`Skipping deleteManyByCredential due to calendar-cache being disabled`);
}

async getCacheStatusByCredentialIds() {
log.info(`Skipping getCacheStatusByCredentialIds due to calendar-cache being disabled`);
return [];
}
}
17 changes: 17 additions & 0 deletions packages/features/calendar-cache/calendar-cache.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,4 +169,21 @@ export class CalendarCacheRepository implements ICalendarCacheRepository {
},
});
}

async getCacheStatusByCredentialIds(credentialIds: number[]) {
const cacheStatuses = await prisma.calendarCache.groupBy({
by: ["credentialId"],
where: {
credentialId: { in: credentialIds },
},
_max: {
updatedAt: true,
},
});

return cacheStatuses.map((cache) => ({
credentialId: cache.credentialId,
updatedAt: cache._max.updatedAt,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Edge Case Handling (confidence: 84%)

When credentialIds is an empty array, the Prisma groupBy query with where: { credentialId: { in: [] } } should return an empty array without error in most Prisma versions, but this is an edge case worth guarding explicitly to prevent potential issues across different database drivers or Prisma versions.

Evidence:

  • The intent specifically calls out: 'If credentialIds array is empty in getCacheStatusByCredentialIds, the groupBy query should return an empty array — verify no SQL errors occur'
  • An empty IN () clause is technically invalid SQL in some databases, though Prisma typically handles this by short-circuiting

Agent: logic

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Edge cases (confidence: 94%)

When credentialIds is an empty array, Prisma's groupBy with { in: [] } may generate an invalid SQL IN () clause on some database engines/versions. While recent Prisma versions may handle this gracefully by short-circuiting, this is database-dependent and not guaranteed. The mock implementation returns [] for empty input, but the real implementation does not guard against it.

Evidence:

  • The intent specification calls this out: 'getCacheStatusByCredentialIds is called even when credentialIds is empty... An empty IN () clause in some databases is invalid — verify Prisma handles empty arrays gracefully'
  • The mock returns [] unconditionally, suggesting the expected behavior for empty input is an empty result
  • connectedCalendarsHandler maps all credential IDs and passes them directly without checking for empty array

Agent: logic

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Grapple PR] Auto-fix — logic agent (Small fix (6 lines, 1 file))

When credentialIds is an empty array, Prisma's groupBy with { in: [] } may generate an invalid SQL IN () clause on some database engines/versions. While recent Prisma versions may handle this gracefully by short-circuiting, this is database-dependent and not guaranteed. The mock implementation returns [] for empty input, but the real implementation does not guard against it.

Suggested change
updatedAt: cache._max.updatedAt,
// Guard against empty array — some DB engines generate invalid `IN ()` SQL.
// The mock implementation also returns [] for empty input, so this is the expected contract.
if (credentialIds.length === 0) {
return [];
}

🤖 Grapple PR auto-fix • major • confidence: 94%

}));
}
}
10 changes: 8 additions & 2 deletions packages/lib/getConnectedDestinationCalendars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,14 @@ type ReturnTypeGetConnectedCalendars = Awaited<ReturnType<typeof getConnectedCal
type ConnectedCalendarsFromGetConnectedCalendars = ReturnTypeGetConnectedCalendars["connectedCalendars"];

export type UserWithCalendars = Pick<User, "id" | "email"> & {
allSelectedCalendars: Pick<SelectedCalendar, "externalId" | "integration" | "eventTypeId">[];
userLevelSelectedCalendars: Pick<SelectedCalendar, "externalId" | "integration" | "eventTypeId">[];
allSelectedCalendars: Pick<
SelectedCalendar,
"externalId" | "integration" | "eventTypeId" | "updatedAt" | "googleChannelId"
>[];
userLevelSelectedCalendars: Pick<
SelectedCalendar,
"externalId" | "integration" | "eventTypeId" | "updatedAt" | "googleChannelId"
>[];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Module boundaries (confidence: 100%)

The UserWithCalendars type now requires updatedAt and googleChannelId fields on selected calendars. This is a shared type used across multiple modules. Adding Google Calendar-specific fields (googleChannelId) to a generic type like UserWithCalendars leaks implementation details of a specific calendar provider into a generic abstraction. Every consumer of this type and every query that builds a UserWithCalendars object must now include these fields, even if they have nothing to do with Google Calendar.

Evidence:

  • The type is exported and used in getConnectedDestinationCalendars which is provider-agnostic
  • Adding googleChannelId to UserWithCalendars couples a generic calendar abstraction to Google-specific schema
  • The user.ts repository query at line 898-899 now selects these fields for all users regardless of their calendar provider

Agent: architecture

destinationCalendar: DestinationCalendar | null;
};

Expand Down
10 changes: 8 additions & 2 deletions packages/lib/server/repository/selectedCalendar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,7 @@ export class SelectedCalendarRepository {
}

static async findMany({ where, select, orderBy }: FindManyArgs) {
const args = { where, select, orderBy } satisfies Prisma.SelectedCalendarFindManyArgs;
return await prisma.selectedCalendar.findMany(args);
return await prisma.selectedCalendar.findMany({ where, select, orderBy });
}

static async findUniqueOrThrow({ where }: { where: Prisma.SelectedCalendarWhereInput }) {
Expand Down Expand Up @@ -398,6 +397,13 @@ export class SelectedCalendarRepository {
});
}

static async updateManyByCredentialId(credentialId: number, data: Prisma.SelectedCalendarUpdateInput) {
return await prisma.selectedCalendar.updateMany({
where: { credentialId },
data,
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Input Validation (confidence: 99%)

updateManyByCredentialId accepts an arbitrary Prisma.SelectedCalendarUpdateInput object with no restrictions, including the empty object {} passed at the call site. Passing {} as the update payload to updateMany may not trigger Prisma's @updatedAt auto-update behavior, silently failing to record the cache refresh timestamp. Additionally, accepting unrestricted Prisma.SelectedCalendarUpdateInput means callers could accidentally (or maliciously, in future misuse) update any SelectedCalendar field for all records under a credential.

Evidence:

  • CalendarService.ts call: await SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {}) — empty data object passed
  • Prisma's @updatedAt directive behavior with empty updateMany payloads is not guaranteed to update the timestamp when no fields change
  • The function signature data: Prisma.SelectedCalendarUpdateInput allows any field to be updated, creating an overly broad API surface
  • PR edge cases explicitly flag: 'if Prisma's @updatedAt directive only triggers on actual field updates, this may not reliably update the timestamp; verify Prisma behavior with empty update payloads'

Agent: security

static async setErrorInWatching({ id, error }: { id: string; error: string }) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Code patterns (confidence: 80%)

updateManyByCredentialId passes empty data object {} which may not trigger Prisma's @updatedat auto-update on SelectedCalendar. The intent appears to be updating the updatedAt timestamp, but empty data may result in a no-op.

Evidence:

  • Line 404: data: Prisma.SelectedCalendarUpdateInput parameter accepts empty object
  • Line 406: data is passed directly to updateMany without forcing updatedAt
  • Prisma @updatedat only auto-updates if other fields are modified, or if explicitly included in data
  • Risk: cache status may not actually update if {} is passed

Agent: style

await SelectedCalendarRepository.updateById(id, {
error,
Expand Down
2 changes: 2 additions & 0 deletions packages/lib/server/repository/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,8 @@ export class UserRepository {
eventTypeId: true,
externalId: true,
integration: true,
updatedAt: true,
googleChannelId: true,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Module Boundaries / Type Expansion (confidence: 80%)

Adding updatedAt and googleChannelId to the SelectedCalendar select in the user repository broadens the data fetched for all consumers of this query, not just the calendar cache feature. This increases the data surface area returned from the database for every call path that uses this select, which may have unintended side effects on other features consuming this user data shape.

Evidence:

  • Lines 899-900 add updatedAt: true and googleChannelId: true to the select clause
  • The UserWithCalendars type in getConnectedDestinationCalendars.ts is also updated to include these fields
  • Any downstream consumer of this user query now receives these additional fields

Agent: architecture

},
},
completedOnboarding: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Link from "next/link";
import React from "react";

import AppListCard from "@calcom/features/apps/components/AppListCard";
import DisconnectIntegration from "@calcom/features/apps/components/DisconnectIntegration";
import CredentialActionsDropdown from "@calcom/features/apps/components/CredentialActionsDropdown";
import AdditionalCalendarSelector from "@calcom/features/calendars/AdditionalCalendarSelector";
import { CalendarSwitch } from "@calcom/features/calendars/CalendarSwitch";
import { useLocale } from "@calcom/lib/hooks/useLocale";
Expand Down Expand Up @@ -67,18 +67,16 @@ const ConnectedCalendarList = ({
description={connectedCalendar.primary?.email ?? connectedCalendar.integration.description}
className="border-subtle mt-4 rounded-lg border"
actions={
// Delegation credential can't be disconnected
!connectedCalendar.delegationCredentialId &&
!disableConnectionModification && (
<div className="flex w-32 justify-end">
<DisconnectIntegration
credentialId={connectedCalendar.credentialId}
trashIcon
onSuccess={onChanged}
buttonProps={{ className: "border border-default" }}
/>
</div>
)
<div className="flex w-32 justify-end">
<CredentialActionsDropdown
credentialId={connectedCalendar.credentialId}
integrationType={connectedCalendar.integration.type}
cacheUpdatedAt={connectedCalendar.cacheUpdatedAt}
onSuccess={onChanged}
delegationCredentialId={connectedCalendar.delegationCredentialId}
disableConnectionModification={disableConnectionModification}
/>
</div>
}>
<div className="border-subtle border-t">
{!fromOnboarding && (

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 INFO — Empty Wrapper DOM Node (confidence: 89%)

The `` wrapper is now unconditionally rendered even when CredentialActionsDropdown returns `null`. Previously, the entire actions block was conditionally rendered. This leaves an empty 32-wide flex div in the DOM when no actions are available (delegation credentials with no cache), potentially causing layout artifacts.

Evidence:

  • Lines 70-82: — no conditional wrapper
  • Old code: !connectedCalendar.delegationCredentialId && !disableConnectionModification && (...) — fully conditional
  • CredentialActionsDropdown returns null when !canDisconnect && !hasCache, but the parent div still occupies space

Agent: security

Expand All @@ -97,7 +95,7 @@ const ConnectedCalendarList = ({
destination={cal.externalId === destinationCalendarId}
credentialId={cal.credentialId}
eventTypeId={shouldUseEventTypeScope ? eventTypeId : null}
delegationCredentialId={connectedCalendar.delegationCredentialId}
delegationCredentialId={connectedCalendar.delegationCredentialId || null}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 INFO — Naming Conventions (confidence: 74%)

Inconsistent handling of delegationCredentialId type coercion using || null. The prop is typed as string | undefined but coerced to string | null, which could mask cases where undefined was previously expected.

Evidence:

  • Line 98: delegationCredentialId={connectedCalendar.delegationCredentialId || null}
  • This changes semantic meaning from 'not provided' (undefined) to 'explicitly null'
  • Could indicate a type mismatch that should be addressed at the source rather than coerced in the component

Agent: style

/>
))}
</ul>
Expand All @@ -122,17 +120,16 @@ const ConnectedCalendarList = ({
}
iconClassName="h-10 w-10 ml-2 mr-1 mt-0.5"
actions={
// Delegation credential can't be disconnected
!connectedCalendar.delegationCredentialId && (
<div className="flex w-32 justify-end">
<DisconnectIntegration
credentialId={connectedCalendar.credentialId}
trashIcon
onSuccess={onChanged}
buttonProps={{ className: "border border-default" }}
/>
</div>
)
<div className="flex w-32 justify-end">
<CredentialActionsDropdown
credentialId={connectedCalendar.credentialId}
integrationType={connectedCalendar.integration.type}
cacheUpdatedAt={connectedCalendar.cacheUpdatedAt}
onSuccess={onChanged}
delegationCredentialId={connectedCalendar.delegationCredentialId}
disableConnectionModification={disableConnectionModification}
/>
</div>
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — State management (confidence: 100%)

In the error/fallback branch (when connectedCalendar.calendars is falsy), CredentialActionsDropdown is rendered without the disableConnectionModification prop. This means even when the parent has disableConnectionModification=true, the error-state card will still allow disconnection, which is inconsistent with the primary branch behavior.

Evidence:

  • Line 70-75 in the primary branch passes disableConnectionModification={disableConnectionModification}
  • Line 120-133 in the error/fallback branch does not pass disableConnectionModification
  • The old code had !connectedCalendar.delegationCredentialId but no disableConnectionModification check in the error branch either, but the new code inconsistently applies it only in one branch

Agent: logic

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Grapple PR] Auto-fix — logic agent (Small fix (1 lines, 1 file))

In the error/fallback branch (when connectedCalendar.calendars is falsy), CredentialActionsDropdown is rendered without the disableConnectionModification prop. This means even when the parent has disableConnectionModification=true, the error-state card will still allow disconnection, which is inconsistent with the primary branch behavior.

Suggested change
}
disableConnectionModification={disableConnectionModification}

🤖 Grapple PR auto-fix • major • confidence: 100%

/>
);
Expand Down Expand Up @@ -162,6 +159,7 @@ export const SelectedCalendarsSettingsWebWrapper = (props: SelectedCalendarsSett
refetchOnWindowFocus: false,
}
);

const { isPending } = props;
const showScopeSelector = !!props.eventTypeId;
const isDisabled = disabledScope ? disabledScope === scope : false;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
Warnings:

- Added the required column `updatedAt` to the `CalendarCache` table without a default value. This is not possible if the table is not empty.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 INFO — Cryptographic issues (confidence: 89%)

The migration file contains a misleading warning comment: 'Added the required column without a default value. This is not possible if the table is not empty.' The actual migration correctly adds DEFAULT NOW(), contradicting the warning. This comment is auto-generated by Prisma and is factually incorrect given the actual SQL. If developers rely on this warning text to understand the migration's safety, they may incorrectly conclude it is unsafe to run on a non-empty table.

Evidence:

  • Lines 3-5: Warning text says 'without a default value' and 'not possible if the table is not empty'
  • Line 9: ALTER TABLE "CalendarCache" ADD COLUMN "updatedAt" TIMESTAMP(3) NOT NULL DEFAULT NOW() — correctly adds a default
  • This contradiction could confuse operators during incident response or deployment reviews
  • PR edge cases note: 'The migration comment warns... but then correctly adds DEFAULT NOW() — the Prisma migration warning text may be misleading'

Agent: security

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 INFO — Migration Safety (confidence: 93%)

The auto-generated Prisma migration warning comment states 'Added the required column updatedAt to the CalendarCache table without a default value. This is not possible if the table is not empty.' However, the actual SQL on line 9 includes DEFAULT NOW(), which safely handles existing rows. The contradictory comment could confuse future developers reviewing migration history.

Evidence:

  • Lines 2-5: Warning says 'without a default value'
  • Line 9: SQL includes DEFAULT NOW()
  • The Prisma schema also includes @default(now()) confirming the default is intentional

Agent: architecture

*/
-- AlterTable
-- Add the column with a default value to safely handle existing rows
ALTER TABLE "CalendarCache" ADD COLUMN "updatedAt" TIMESTAMP(3) NOT NULL DEFAULT NOW();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Migration Safety (confidence: 100%)

Using DEFAULT NOW() in the migration means all existing CalendarCache rows will get the current timestamp as their updatedAt value, which incorrectly suggests they were recently updated. While this is noted as a risk area and is safe from a data-loss perspective, it creates misleading cache status information for existing entries.

Evidence:

  • Risk area from intent: 'existing rows will get NOW() as their timestamp, which may incorrectly suggest recent cache activity for stale data'
  • Users may see 'Last updated: ' for cache entries that are actually much older
  • This is a one-time issue that self-corrects as cache is refreshed, but could cause initial confusion
  • Line 9: ALTER TABLE "CalendarCache" ADD COLUMN "updatedAt" TIMESTAMP(3) NOT NULL DEFAULT NOW();
  • Existing stale cache rows will appear as 'recently updated' in the UI immediately after migration
  • The CredentialActionsDropdown displays this timestamp to users as 'Last updated: ...'

Agent: logic

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Misleading Migration Warning / Documentation Integrity (confidence: 100%)

The migration file contains a Prisma-generated warning comment stating 'Added the required column updatedAt to the CalendarCache table without a default value. This is not possible if the table is not empty.' However, the actual SQL immediately below adds the column WITH DEFAULT NOW(). This contradiction between the warning and the implementation will confuse future developers who read the migration, potentially causing them to distrust or misinterpret other migration warnings. The comment should be updated or removed to accurately reflect that a default value IS provided.

Evidence:

  • Line 4: '...without a default value. This is not possible if the table is not empty.'
  • Line 9: ADD COLUMN "updatedAt" TIMESTAMP(3) NOT NULL DEFAULT NOW(); — contradicts the warning
  • The review context explicitly notes this contradiction: 'The migration comment warns... but the actual SQL adds DEFAULT NOW(), creating a contradiction'
  • Line 5: 'without a default value. This is not possible if the table is not empty.'
  • Line 9: 'ADD COLUMN "updatedAt" TIMESTAMP(3) NOT NULL DEFAULT NOW()'
  • The edge cases flag this: 'creates a contradiction in the generated migration warning text that could confuse future developers'
  • Lines 2-3 warn: 'without a default value. This is not possible if the table is not empty.'
  • Line 9 actual SQL: 'DEFAULT NOW()' is present
  • This contradiction could confuse maintainers about the actual safety of the migration

Agent: security

2 changes: 2 additions & 0 deletions packages/prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -1715,6 +1715,8 @@ model CalendarCache {
key String
value Json
expiresAt DateTime
// Provide an initial value for legacy rows and future raw inserts
updatedAt DateTime @default(now()) @updatedAt

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 INFO — Documentation (confidence: 77%)

Comment mentions 'legacy rows' but the migration adds DEFAULT NOW() which timestamps existing rows with current time, potentially misrepresenting when they were actually cached. This could be misleading to future maintainers.

Evidence:

  • Schema comment says 'Provide an initial value for legacy rows'
  • Migration uses DEFAULT NOW() which gives all existing rows the current timestamp
  • Existing stale cache entries will appear to have been recently updated
  • This timing inconsistency should be documented

Agent: style

credentialId Int
userId Int?
credential Credential? @relation(fields: [credentialId], references: [id], onDelete: Cascade)
Expand Down
9 changes: 9 additions & 0 deletions packages/trpc/server/routers/viewer/calendars/_router.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { z } from "zod";

import authedProcedure from "../../../procedures/authedProcedure";
import { router } from "../../../trpc";
import { ZConnectedCalendarsInputSchema } from "./connectedCalendars.schema";
Expand All @@ -22,4 +24,11 @@ export const calendarsRouter = router({

return setDestinationCalendarHandler({ ctx, input });
}),

deleteCache: authedProcedure
.input(z.object({ credentialId: z.number() }))
.mutation(async ({ ctx, input }) => {
const { deleteCacheHandler } = await import("./deleteCache.handler");
return deleteCacheHandler({ ctx, input });
}),
});
Loading