Feat/calendar sync#6
Conversation
…cal.com into feat/calendar_sync_cache
- Add tests for /api/cron/calendar-subscriptions-cleanup route (9 tests) - Add tests for /api/cron/calendar-subscriptions route (10 tests) - Add tests for /api/webhooks/calendar-subscription/[provider] route (11 tests) - Add missing feature flags for calendar-subscription-cache and calendar-subscription-sync - All 30 tests pass with comprehensive coverage of authentication, feature flags, error handling, and service instantiation Tests cover: - Authentication scenarios (API key validation, Bearer tokens, query parameters) - Feature flag combinations (cache/sync enabled/disabled states) - Success and error handling (including non-Error exceptions) - Service instantiation with proper dependency injection - Provider validation for webhook endpoints Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
…ositories, and adapters - Add unit tests for CalendarSubscriptionService with subscription, webhook, and event processing - Add unit tests for CalendarCacheEventService with cache operations and cleanup - Add unit tests for CalendarSyncService with Cal.com event filtering and booking operations - Add unit tests for CalendarCacheEventRepository with CRUD operations - Add unit tests for SelectedCalendarRepository with calendar selection management - Add unit tests for GoogleCalendarSubscriptionAdapter with subscription and event fetching - Add unit tests for Office365CalendarSubscriptionAdapter with placeholder implementation - Add unit tests for AdaptersFactory with provider management and adapter creation - Fix lint issues by removing explicit 'any' type casting and unused variables - All tests follow Cal.com conventions using Vitest framework with proper mocking Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
…g missing mocks - Add comprehensive mocks for defaultResponderForAppDir, logger, performance monitoring, and Sentry - Fix slow test execution (933ms -> <100ms) caused by missing dependency mocks - Ensure consistent test performance across different environments Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
Co-authored-by: Alex van Andel <me@alexvanandel.com>
|
@codity review |
Policy Check Failed✗ 2/3 policy checks failed: • Need 2 more approval(s) (0/2) — comment LGTM or approve via review ✓ 1 checks passed: To merge this PR:
|
PR SummaryWhat Changed
Key Changes by AreaCalendar Sync: New Booking Cancellation: Schema: Added Files Changed
Review Focus Areas
ArchitectureDesign Decisions: The explicit flag pattern ( Scalability & Extensibility: Out of scope. Current implementation handles Google/Office365 only. Adding new calendar providers requires extending Risks:
Merge StatusMERGEABLE — PR Score 71/100, above threshold (50). All gates passed. |
![codity-dm[bot]](https://avatars.githubusercontent.com/u/234452856?s=60&v=4){kind=small}
| await handleCancelBooking({ | ||
| userId: booking.userId, | ||
| bookingData: { | ||
| uid: booking.uid, | ||
| cancellationReason: "Cancelled on user's calendar", | ||
| cancelledBy: booking.userPrimaryEmail, | ||
| // Skip calendar event deletion to avoid infinite loops | ||
| // (Google/Office365 → Cal.com → Google/Office365 → ...) | ||
| skipCalendarSyncTaskCancellation: true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
handleCancelBooking is called without an actionSource, so it defaults to "UNKNOWN" inside the handler. The handler explicitly checks for this and emits log.warn("Booking cancellation with unknown actionSource", ...) on every single calendar-sync cancellation. "WEBHOOK" is the closest semantic match in the existing ActionSource enum (API_V1 | API_V2 | WEBAPP | WEBHOOK | MAGIC_LINK | UNKNOWN) and should be passed here so audit records are correctly attributed and the warning is not emitted.
Suggested fix
await handleCancelBooking({
userId: booking.userId,
actionSource: "WEBHOOK",
bookingData: {
uid: booking.uid,
cancellationReason: "Cancelled on user's calendar",
cancelledBy: booking.userPrimaryEmail,
// Skip calendar event deletion to avoid infinite loops
// (Google/Office365 → Cal.com → Google/Office365 → ...)
skipCalendarSyncTaskCancellation: true,
},
});Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert typescript developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts
Lines: 105-115
Issue Type: functional-medium
Severity: medium
Issue Description:
`handleCancelBooking` is called without an `actionSource`, so it defaults to `"UNKNOWN"` inside the handler. The handler explicitly checks for this and emits `log.warn("Booking cancellation with unknown actionSource", ...)` on every single calendar-sync cancellation. `"WEBHOOK"` is the closest semantic match in the existing `ActionSource` enum (`API_V1 | API_V2 | WEBAPP | WEBHOOK | MAGIC_LINK | UNKNOWN`) and should be passed here so audit records are correctly attributed and the warning is not emitted.
Current Code:
await handleCancelBooking({
userId: booking.userId,
bookingData: {
uid: booking.uid,
cancellationReason: "Cancelled on user's calendar",
cancelledBy: booking.userPrimaryEmail,
// Skip calendar event deletion to avoid infinite loops
// (Google/Office365 → Cal.com → Google/Office365 → ...)
skipCalendarSyncTaskCancellation: true,
},
});
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow typescript best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| import logger from "@calcom/lib/logger"; | ||
| import { safeStringify } from "@calcom/lib/safeStringify"; | ||
| import type { SelectedCalendar } from "@calcom/prisma/client"; | ||
| import { metrics } from "@sentry/nextjs"; |
There was a problem hiding this comment.
CalendarSyncService lives in a shared packages/features package but statically imports { metrics } from @sentry/nextjs, a Next.js-specific SDK. This hard-couples the module to the Next.js runtime and will fail to initialize correctly (or outright throw) when the package is loaded in a standalone Node.js server, a background worker, or any non-Next.js consumer. The same file already avoids this problem for RegularBookingService.container via a dynamic import() — the Sentry metrics import should follow the same pattern or use the provider-agnostic @sentry/core package.
| import { metrics } from "@sentry/nextjs"; | |
| import { metrics } from "@sentry/core"; |
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert typescript developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts
Lines: 7-7
Issue Type: maintainability-medium
Severity: medium
Issue Description:
`CalendarSyncService` lives in a shared `packages/features` package but statically imports `{ metrics }` from `@sentry/nextjs`, a Next.js-specific SDK. This hard-couples the module to the Next.js runtime and will fail to initialize correctly (or outright throw) when the package is loaded in a standalone Node.js server, a background worker, or any non-Next.js consumer. The same file already avoids this problem for `RegularBookingService.container` via a dynamic `import()` — the Sentry metrics import should follow the same pattern or use the provider-agnostic `@sentry/core` package.
Current Code:
import { metrics } from "@sentry/nextjs";
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow typescript best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Security Scan Summary
No critical security issues detected Scan completed in 12.2sSecurity scan powered by Codity.ai |
License Compliance Scan
Weak copyleft licenses found - verify compatibility Some packages have unknown licenses - manual review required Medium Risk Licenses - 5 packages(MPL-2.0 OR Apache-2.0) (1 packages):
MPL-2.0 (4 packages):
Unknown Licenses - 44 packages
...and 24 more Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/cal.com · PR #6Scanned: 2026-05-24 19:49 UTC | Score: 52/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-001]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
packages/features/bookings/lib/handleCancelBooking.ts |
0 | 1 | 1 | 0 | 2 |
packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts |
0 | 0 | 2 | 1 | 3 |
packages/features/calendar-subscription/lib/sync/__tests__/CalendarSyncService.test.ts |
0 | 0 | 0 | 14 | 14 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
|
@codity review |
Policy Check Failed✗ 2/3 policy checks failed: • Need 2 more approval(s) (0/2) — comment LGTM or approve via review ✓ 1 checks passed: To merge this PR:
|
|
PR review started! Estimated time: 5-10 minutes. Learn MoreAsk Codity questions: Mention Trigger a manual review: Comment Generate unit tests: Comment Run security scan again: Comment |
| vi.mock("@calcom/features/bookings/lib/handleCancelBooking", () => ({ | ||
| default: mockHandleCancelBooking, | ||
| })); | ||
|
|
||
| vi.mock("@calcom/features/bookings/di/RegularBookingService.container", () => ({ | ||
| getRegularBookingService: () => ({ | ||
| createBooking: mockCreateBooking, | ||
| }), | ||
| })); |
There was a problem hiding this comment.
CalendarSyncService calls metrics.count(...) and metrics.distribution(...) from @sentry/nextjs as the very first statement inside handleEvents, cancelBooking, and rescheduleBooking — before any try/catch. The sibling test file CalendarSubscriptionService.test.ts already carries a vi.mock('@sentry/nextjs', ...) for exactly this reason, but this new test file omits it entirely. Without the mock, every test that invokes any of those three methods will throw TypeError: Cannot read properties of undefined (reading 'count') and the entire suite will fail at runtime.
Suggested fix
vi.mock("@calcom/features/bookings/lib/handleCancelBooking", () => ({
default: mockHandleCancelBooking,
}));
vi.mock("@calcom/features/bookings/di/RegularBookingService.container", () => ({
getRegularBookingService: () => ({
createBooking: mockCreateBooking,
}),
}));
vi.mock("@sentry/nextjs", () => ({
metrics: {
count: vi.fn(),
distribution: vi.fn(),
},
}));Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert typescript developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/__tests__/CalendarSyncService.test.ts
Lines: 14-22
Issue Type: functional-high
Severity: high
Issue Description:
`CalendarSyncService` calls `metrics.count(...)` and `metrics.distribution(...)` from `@sentry/nextjs` as the very first statement inside `handleEvents`, `cancelBooking`, and `rescheduleBooking` — before any try/catch. The sibling test file `CalendarSubscriptionService.test.ts` already carries a `vi.mock('@sentry/nextjs', ...)` for exactly this reason, but this new test file omits it entirely. Without the mock, every test that invokes any of those three methods will throw `TypeError: Cannot read properties of undefined (reading 'count')` and the entire suite will fail at runtime.
Current Code:
vi.mock("@calcom/features/bookings/lib/handleCancelBooking", () => ({
default: mockHandleCancelBooking,
}));
vi.mock("@calcom/features/bookings/di/RegularBookingService.container", () => ({
getRegularBookingService: () => ({
createBooking: mockCreateBooking,
}),
}));
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow typescript best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| const booking = await this.deps.bookingRepository.findBookingByUidWithEventType({ bookingUid }); | ||
| if (!booking) { | ||
| log.debug("Unable to sync, booking not found"); | ||
| log.debug("Unable to sync, booking not found in database", { bookingUid }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The bookingRepository.findBookingByUidWithEventType call in cancelBooking (and its mirror in rescheduleBooking at line ~152) sits outside the try/catch block. When this DB call throws — e.g. on a transient connection error — the exception propagates up to Promise.all in handleEvents. Promise.all fails fast on the first rejection, so any other calendar events in the same batch that haven't started yet are silently skipped. A single flaky DB call can therefore cause an entire sync batch to be partially applied with no retry and no per-event error logged.
Suggested fix
let booking;
try {
booking = await this.deps.bookingRepository.findBookingByUidWithEventType({ bookingUid });
} catch (error) {
log.error("Unable to sync, failed to fetch booking from database", {
bookingUid,
error: safeStringify(error),
});
return;
}
if (!booking) {
log.debug("Unable to sync, booking not found in database", { bookingUid });
return;
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert typescript developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts
Lines: 86-90
Issue Type: robustness-medium
Severity: medium
Issue Description:
The `bookingRepository.findBookingByUidWithEventType` call in `cancelBooking` (and its mirror in `rescheduleBooking` at line ~152) sits outside the try/catch block. When this DB call throws — e.g. on a transient connection error — the exception propagates up to `Promise.all` in `handleEvents`. `Promise.all` fails fast on the first rejection, so any other calendar events in the same batch that haven't started yet are silently skipped. A single flaky DB call can therefore cause an entire sync batch to be partially applied with no retry and no per-event error logged.
Current Code:
const booking = await this.deps.bookingRepository.findBookingByUidWithEventType({ bookingUid });
if (!booking) {
log.debug("Unable to sync, booking not found in database", { bookingUid });
return;
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow typescript best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
|
@codity review |
Policy Check Failed✗ 2/3 policy checks failed: • Need 2 more approval(s) (0/2) — comment LGTM or approve via review ✓ 1 checks passed: To merge this PR:
|
PR SummaryWhat Changed
Key Changes by AreaCalendar Sync Service: New Booking Cancellation: Updated Schema Validation: Added Files Changed
Review Focus Areas
ArchitectureDesign Decisions: The sync service uses iCalUID suffix matching to identify Cal.com bookings rather than external IDs. This is a pragmatic choice given iCal format constraints, though it assumes Cal.com UID conventions are stable. Risks:
Merge StatusMERGEABLE — PR Score 55/100, above threshold (50). All gates passed. |
| cancelSubsequentBookings: z.boolean().optional(), | ||
| cancellationReason: z.string().optional(), | ||
| skipCancellationReasonValidation: z.boolean().optional(), | ||
| skipCalendarSyncTaskCancellation: z.boolean().optional(), |
There was a problem hiding this comment.
The skipCalendarSyncTaskCancellation flag is added to bookingCancelSchema, which is extended by bookingCancelWithCsrfSchema (line 415). That CSRF schema is parsed verbatim in apps/web/app/api/cancel/route.ts (line 23) and the resulting object is passed directly to handleCancelBooking without stripping internal fields. Any authenticated user with a valid CSRF token can therefore POST { uid: "...", csrfToken: "...", skipCalendarSyncTaskCancellation: true } and bypass calendar event deletion entirely: causing their provider calendar (Google/Office365) to retain the event while Cal.com marks the booking cancelled and its references deleted. The flag was designed as an internal circuit-breaker, but it has leaked into the public API surface. Fix: remove the field from bookingCancelSchema and instead thread it through the CancelBookingInput TypeScript type (which is never directly serialized from user input) so it cannot be supplied by external callers.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/prisma/zod-utils.ts
Lines: 393-393
Issue Type: security-high
Severity: high
Issue Description:
The `skipCalendarSyncTaskCancellation` flag is added to `bookingCancelSchema`, which is extended by `bookingCancelWithCsrfSchema` (line 415). That CSRF schema is parsed verbatim in `apps/web/app/api/cancel/route.ts` (line 23) and the resulting object is passed directly to `handleCancelBooking` without stripping internal fields. Any authenticated user with a valid CSRF token can therefore POST `{ uid: "...", csrfToken: "...", skipCalendarSyncTaskCancellation: true }` and bypass calendar event deletion entirely: causing their provider calendar (Google/Office365) to retain the event while Cal.com marks the booking cancelled and its references deleted. The flag was designed as an internal circuit-breaker, but it has leaked into the public API surface. Fix: remove the field from `bookingCancelSchema` and instead thread it through the `CancelBookingInput` TypeScript type (which is never directly serialized from user input) so it cannot be supplied by external callers.
Current Code:
skipCalendarSyncTaskCancellation: z.boolean().optional(),
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| try { | ||
| await bookingReferenceRepository.updateManyByBookingId(bookingToDelete.id, { deleted: true }); | ||
| } catch (error) { | ||
| log.error(`Error deleting integrations`, safeStringify({ error })); | ||
| log.error(`Error marking booking references as deleted`, safeStringify({ error })); | ||
| } |
There was a problem hiding this comment.
In the original code, bookingReferenceRepository.updateManyByBookingId(..., { deleted: true }) lived inside the same try block as eventManager.cancelEvent. If cancelEvent threw (e.g. network timeout, OAuth revocation), the catch swallowed the error and references were NOT marked deleted, leaving them in a state where a retry could re-attempt the calendar API call. This refactor moves reference deletion into a separate always-executed try/catch, so references are now marked deleted: true even when cancelEvent fails. The comment at line 649 justifies this only for the sync path ("external event is already deleted"), but the block also runs on the normal cancellation path where the calendar event may genuinely still exist on the provider. A failed calendar delete followed by a reference being marked deleted creates a phantom orphaned calendar event that cannot be cleaned up through normal retry paths. The behavioural change should either be explicitly bounded to skipCalendarSyncTaskCancellation === true, or the comment should acknowledge and justify the normal-path change.
Suggested fix
if (skipCalendarSyncTaskCancellation) {
// Sync path: external event is already deleted on the provider calendar;
// always mark references deleted for consistency.
try {
await bookingReferenceRepository.updateManyByBookingId(bookingToDelete.id, { deleted: true });
} catch (error) {
log.error(`Error marking booking references as deleted`, safeStringify({ error }));
}
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/bookings/lib/handleCancelBooking.ts
Lines: 650-654
Issue Type: functional-medium
Severity: medium
Issue Description:
In the original code, `bookingReferenceRepository.updateManyByBookingId(..., { deleted: true })` lived inside the same try block as `eventManager.cancelEvent`. If `cancelEvent` threw (e.g. network timeout, OAuth revocation), the catch swallowed the error and references were NOT marked deleted, leaving them in a state where a retry could re-attempt the calendar API call. This refactor moves reference deletion into a separate always-executed try/catch, so references are now marked `deleted: true` even when `cancelEvent` fails. The comment at line 649 justifies this only for the sync path ("external event is already deleted"), but the block also runs on the normal cancellation path where the calendar event may genuinely still exist on the provider. A failed calendar delete followed by a reference being marked deleted creates a phantom orphaned calendar event that cannot be cleaned up through normal retry paths. The behavioural change should either be explicitly bounded to `skipCalendarSyncTaskCancellation === true`, or the comment should acknowledge and justify the normal-path change.
Current Code:
try {
await bookingReferenceRepository.updateManyByBookingId(bookingToDelete.id, { deleted: true });
} catch (error) {
log.error(`Error marking booking references as deleted`, safeStringify({ error }));
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| }); | ||
| } catch (error) { | ||
| // Log error but don't block - calendar change should still be reflected | ||
| log.error("Failed to cancel booking from calendar sync", { bookingUid, error: safeStringify(error) }); |
There was a problem hiding this comment.
handleCancelBooking throws HttpError (4xx) for several expected, non-fatal conditions that will commonly occur during calendar sync: "This booking has already been cancelled" (idempotent re-delivery of the same webhook), "Cannot cancel a booking that has already ended" (user deletes a past event), "This event type does not allow cancellations". All of these are caught here and emitted as log.error, indistinguishable from genuine system failures (DB down, unhandled exception). Operators monitoring error-level logs or Sentry issues will see constant noise from routine sync scenarios, masking real failures and causing alert fatigue. The catch should distinguish 4xx HttpError instances (log at warn/info) from actual system errors (log at error).
Suggested fix
const isExpectedClientError =
typeof (error as { statusCode?: number })?.statusCode === "number" &&
(error as { statusCode: number }).statusCode >= 400 &&
(error as { statusCode: number }).statusCode < 500;
if (isExpectedClientError) {
log.warn("Calendar sync cancellation skipped due to booking validation", {
bookingUid,
message: String((error as Error).message),
});
} else {
log.error("Failed to cancel booking from calendar sync", { bookingUid, error: safeStringify(error) });
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts
Lines: 126-126
Issue Type: robustness-medium
Severity: medium
Issue Description:
`handleCancelBooking` throws `HttpError` (4xx) for several expected, non-fatal conditions that will commonly occur during calendar sync: "This booking has already been cancelled" (idempotent re-delivery of the same webhook), "Cannot cancel a booking that has already ended" (user deletes a past event), "This event type does not allow cancellations". All of these are caught here and emitted as `log.error`, indistinguishable from genuine system failures (DB down, unhandled exception). Operators monitoring error-level logs or Sentry issues will see constant noise from routine sync scenarios, masking real failures and causing alert fatigue. The catch should distinguish 4xx `HttpError` instances (log at warn/info) from actual system errors (log at error).
Current Code:
log.error("Failed to cancel booking from calendar sync", { bookingUid, error: safeStringify(error) });
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| start: event.start?.toISOString() ?? booking.startTime.toISOString(), | ||
| end: event.end?.toISOString() ?? booking.endTime.toISOString(), | ||
| timeZone: event.timeZone ?? "UTC", | ||
| language: "en", |
There was a problem hiding this comment.
rescheduleBooking passes a hardcoded language: "en" to createBooking. The booking service uses this language to look up translations for confirmation emails sent to all attendees. Attendees of non-English bookings will receive reschedule notifications in English regardless of their locale preference. The user's locale is available on the booking object (if selected in the repository query) or can default to the event's attendee locale. At minimum, this should fall back to the booking owner's locale rather than always forcing English.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts
Lines: 177-177
Issue Type: functional-medium
Severity: medium
Issue Description:
`rescheduleBooking` passes a hardcoded `language: "en"` to `createBooking`. The booking service uses this language to look up translations for confirmation emails sent to all attendees. Attendees of non-English bookings will receive reschedule notifications in English regardless of their locale preference. The user's locale is available on the `booking` object (if selected in the repository query) or can default to the event's attendee locale. At minimum, this should fall back to the booking owner's locale rather than always forcing English.
Current Code:
language: "en",
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| import logger from "@calcom/lib/logger"; | ||
| import { safeStringify } from "@calcom/lib/safeStringify"; | ||
| import type { SelectedCalendar } from "@calcom/prisma/client"; | ||
| import { metrics } from "@sentry/nextjs"; |
There was a problem hiding this comment.
CalendarSyncService lives in packages/features/calendar-subscription/, a shared feature package that is (or may be) consumed from workers, cron jobs, or other non-Next.js runtimes. Importing { metrics } from @sentry/nextjs rather than a framework-agnostic package like @sentry/core couples this service to the Next.js Sentry integration. @sentry/nextjs monkey-patches Next.js internals at import time and may fail or behave unpredictably outside a Next.js request context. Using the framework-neutral @sentry/core (which @sentry/nextjs re-exports for this purpose) removes the runtime constraint.
| import { metrics } from "@sentry/nextjs"; | |
| import { metrics } from "@sentry/core"; |
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts
Lines: 7-7
Issue Type: maintainability-medium
Severity: medium
Issue Description:
`CalendarSyncService` lives in `packages/features/calendar-subscription/`, a shared feature package that is (or may be) consumed from workers, cron jobs, or other non-Next.js runtimes. Importing `{ metrics }` from `@sentry/nextjs` rather than a framework-agnostic package like `@sentry/core` couples this service to the Next.js Sentry integration. `@sentry/nextjs` monkey-patches Next.js internals at import time and may fail or behave unpredictably outside a Next.js request context. Using the framework-neutral `@sentry/core` (which `@sentry/nextjs` re-exports for this purpose) removes the runtime constraint.
Current Code:
import { metrics } from "@sentry/nextjs";
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
There was a problem hiding this comment.
Promise.all fans out all filtered Cal.com events concurrently with no concurrency cap. A calendar diff with 50: 100 changed events (e.g. after a long offline period or a full re-sync) launches that many simultaneous handleCancelBooking or createBooking calls, each of which opens multiple Prisma queries, sends emails, fires webhooks, and calls third-party calendar APIs. This can exhaust the Prisma connection pool, trigger rate-limits on downstream services, and spike DB load for every subscriber that syncs at the same time. Sequential processing or a bounded concurrency limit (e.g. p-limit with 5 or less) would keep throughput acceptable while protecting downstream resources.
Suggested fix
for (const e of calEvents) {
if (e.status === "cancelled") {
await this.cancelBooking(e);
} else {
await this.rescheduleBooking(e);
}
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts
Lines: 61-69
Issue Type: performance-medium
Severity: medium
Issue Description:
`Promise.all` fans out all filtered Cal.com events concurrently with no concurrency cap. A calendar diff with 50: 100 changed events (e.g. after a long offline period or a full re-sync) launches that many simultaneous `handleCancelBooking` or `createBooking` calls, each of which opens multiple Prisma queries, sends emails, fires webhooks, and calls third-party calendar APIs. This can exhaust the Prisma connection pool, trigger rate-limits on downstream services, and spike DB load for every subscriber that syncs at the same time. Sequential processing or a bounded concurrency limit (e.g. p-limit with 5 or less) would keep throughput acceptable while protecting downstream resources.
Current Code:
await Promise.all(
calEvents.map((e) => {
if (e.status === "cancelled") {
return this.cancelBooking(e);
} else {
return this.rescheduleBooking(e);
}
})
);
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Nitpicks (Low Priority)Found 1 low-priority suggestions for code improvement Click to expand nitpicks
|
Security Scan Summary
No critical security issues detected Scan completed in 14.0sSecurity scan powered by Codity.ai |
License Compliance Scan
Weak copyleft licenses found - verify compatibility Some packages have unknown licenses - manual review required Medium Risk Licenses - 5 packages(MPL-2.0 OR Apache-2.0) (1 packages):
MPL-2.0 (4 packages):
Unknown Licenses - 44 packages
...and 24 more Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/cal.com · PR #6Scanned: 2026-05-31 09:02 UTC | Score: 52/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-001]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
packages/features/bookings/lib/handleCancelBooking.ts |
0 | 1 | 1 | 0 | 2 |
packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts |
0 | 0 | 2 | 1 | 3 |
packages/features/calendar-subscription/lib/sync/__tests__/CalendarSyncService.test.ts |
0 | 0 | 0 | 14 | 14 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
|
@codity review |
Policy Check Failed✗ 2/3 policy checks failed: • Need 2 more approval(s) (0/2) — comment LGTM or approve via review ✓ 1 checks passed: To merge this PR:
|
PR SummaryWhat Changed
Key Changes by AreaCalendar Sync Service: New Booking Cancellation: Extended Schema: Added Files Changed
Review Focus Areas
ArchitectureDesign Decisions: Dynamic imports avoid loading Risks: Intentional: Non-blocking error handling means sync failures are silent. Unintentional: Mixed-case iCalUID matching could miss events if external calendars normalize differently. Merge StatusNOT MERGEABLE — PR Score 41/100, below threshold (50)
|
Workflow DiagramsAutomatically generated sequence diagrams showing the workflows in this PR 1. Calendar Sync Bidirectional Flow with Loop PreventionComplex complexity • Components: CalendarSyncService, handleCancelBooking, RegularBookingService sequenceDiagram
title Bidirectional Calendar Synchronization with Loop Prevention
participant ExternalCalendar as External Calendar
participant CalendarSyncService as CalendarSyncService
participant BookingRepository as BookingRepository
participant handleCancelBooking as handleCancelBooking
participant RegularBookingService as RegularBookingService
participant EventManager as EventManager
participant Database as Database
ExternalCalendar->>CalendarSyncService: Webhook: calendar event changed
Note over CalendarSyncService: Filter events with iCalUID ending in @cal.com
alt Event status is cancelled
CalendarSyncService->>BookingRepository: findBookingByUidWithEventType(bookingUid)
BookingRepository-->>CalendarSyncService: booking data
alt Booking found with required user data
CalendarSyncService->>handleCancelBooking: cancel booking with skipCalendarSyncTaskCancellation=true
Note over handleCancelBooking: Skip calendar event deletion to prevent infinite loop
handleCancelBooking->>Database: Mark booking references as deleted
handleCancelBooking-->>CalendarSyncService: cancellation complete
end
else Event status is confirmed or updated
CalendarSyncService->>BookingRepository: findBookingByUidWithEventType(bookingUid)
BookingRepository-->>CalendarSyncService: booking data
alt Booking found with eventTypeId
CalendarSyncService->>RegularBookingService: createBooking with rescheduleUid and skipCalendarSyncTaskCreation=true
Note over RegularBookingService: Skip calendar event creation to prevent infinite loop
RegularBookingService->>Database: Create rescheduled booking
RegularBookingService-->>CalendarSyncService: reschedule complete
end
end
Note over CalendarSyncService: Record Sentry metrics for monitoring
Note: Diagrams show detected patterns only. Complex workflows may require manual review. |
| cancelSubsequentBookings: z.boolean().optional(), | ||
| cancellationReason: z.string().optional(), | ||
| skipCancellationReasonValidation: z.boolean().optional(), | ||
| skipCalendarSyncTaskCancellation: z.boolean().optional(), |
There was a problem hiding this comment.
The skipCalendarSyncTaskCancellation flag is added to bookingCancelSchema, which is the base for bookingCancelWithCsrfSchema used directly in the public /api/cancel route (apps/web/app/api/cancel/route.ts:23). Any authenticated user can include skipCalendarSyncTaskCancellation: true in their POST body to bypass calendar synchronization on cancellation. This is an internal-only escape hatch that should never be exposed to client requests: it should be stripped at the API boundary or live on an internal-only schema that is never merged with the public one.
Suggested fix
// NOTE: skipCalendarSyncTaskCancellation is intentionally omitted here;
// it is only set programmatically by CalendarSyncService and must not be
// accepted from untrusted request bodies.Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/prisma/zod-utils.ts
Lines: 393-393
Issue Type: security-high
Severity: high
Issue Description:
The `skipCalendarSyncTaskCancellation` flag is added to `bookingCancelSchema`, which is the base for `bookingCancelWithCsrfSchema` used directly in the public `/api/cancel` route (`apps/web/app/api/cancel/route.ts:23`). Any authenticated user can include `skipCalendarSyncTaskCancellation: true` in their POST body to bypass calendar synchronization on cancellation. This is an internal-only escape hatch that should never be exposed to client requests: it should be stripped at the API boundary or live on an internal-only schema that is never merged with the public one.
Current Code:
skipCalendarSyncTaskCancellation: z.boolean().optional(),
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| try { | ||
| await bookingReferenceRepository.updateManyByBookingId(bookingToDelete.id, { deleted: true }); | ||
| } catch (error) { | ||
| log.error(`Error deleting integrations`, safeStringify({ error })); | ||
| log.error(`Error marking booking references as deleted`, safeStringify({ error })); | ||
| } |
There was a problem hiding this comment.
The refactor separates bookingReferenceRepository.updateManyByBookingId from the eventManager.cancelEvent try-block, so it now runs unconditionally even when cancelEvent throws. In the original code both calls lived in the same try-block: if cancelEvent failed the references were NOT marked deleted, keeping the DB consistent with the external calendar state. Now, if cancelEvent fails on the normal (non-skip) path, the booking references in cal.com are marked deleted while the actual calendar event may still exist: creating a silent inconsistency that is very hard to detect or reconcile later.
Suggested fix
// When skipping calendar event deletion the external event was already removed;
// mark references deleted for consistency. When NOT skipping, only mark
// references deleted after a successful cancelEvent.
if (skipCalendarSyncTaskCancellation) {
try {
await bookingReferenceRepository.updateManyByBookingId(bookingToDelete.id, { deleted: true });
} catch (error) {
log.error(`Error marking booking references as deleted`, safeStringify({ error }));
}
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/bookings/lib/handleCancelBooking.ts
Lines: 650-654
Issue Type: functional-high
Severity: high
Issue Description:
The refactor separates `bookingReferenceRepository.updateManyByBookingId` from the `eventManager.cancelEvent` try-block, so it now runs unconditionally even when `cancelEvent` throws. In the original code both calls lived in the same try-block: if `cancelEvent` failed the references were NOT marked deleted, keeping the DB consistent with the external calendar state. Now, if `cancelEvent` fails on the normal (non-skip) path, the booking references in cal.com are marked deleted while the actual calendar event may still exist: creating a silent inconsistency that is very hard to detect or reconcile later.
Current Code:
// Always mark booking references as deleted for data consistency
// (even when skipCalendarSyncTaskCancellation is true, since the external event is already deleted)
try {
await bookingReferenceRepository.updateManyByBookingId(bookingToDelete.id, { deleted: true });
} catch (error) {
log.error(`Error marking booking references as deleted`, safeStringify({ error }));
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| import { beforeEach, describe, expect, test, vi } from "vitest"; | ||
|
|
||
| import type { CalendarSubscriptionEventItem } from "@calcom/features/calendar-subscription/lib/CalendarSubscriptionPort.interface"; | ||
| import type { BookingRepository } from "@calcom/lib/server/repository/booking"; |
There was a problem hiding this comment.
The test imports BookingRepository from @calcom/lib/server/repository/booking, which does not exist anywhere in the repository (confirmed by filesystem inspection: no file at packages/lib/server/repository/booking.ts or any booking export under that path). The production code imports from @calcom/features/bookings/repositories/BookingRepository. This broken import will cause a TypeScript compilation error, preventing CI from passing.
| import type { BookingRepository } from "@calcom/lib/server/repository/booking"; | |
| import type { BookingRepository } from "@calcom/features/bookings/repositories/BookingRepository"; |
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/__tests__/CalendarSyncService.test.ts
Lines: 4-4
Issue Type: functional-medium
Severity: medium
Issue Description:
The test imports `BookingRepository` from `@calcom/lib/server/repository/booking`, which does not exist anywhere in the repository (confirmed by filesystem inspection: no file at `packages/lib/server/repository/booking.ts` or any `booking` export under that path). The production code imports from `@calcom/features/bookings/repositories/BookingRepository`. This broken import will cause a TypeScript compilation error, preventing CI from passing.
Current Code:
import type { BookingRepository } from "@calcom/lib/server/repository/booking";
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| status: "cancelled", | ||
| }; | ||
|
|
||
| describe("CalendarSyncService", () => { |
There was a problem hiding this comment.
The test file does not mock @sentry/nextjs, but CalendarSyncService calls metrics.count and metrics.distribution on every code path. Every sibling test in this package (CalendarSubscriptionService.test.ts) explicitly mocks @sentry/nextjs. Without the mock the tests either fail (if Sentry throws in the test environment) or silently call through to the real SDK (potentially making network requests or causing test flakes). Add a vi.mock("@sentry/nextjs", ...) at the top of the file, matching the pattern used in the sibling test.
Suggested fix
vi.mock("@sentry/nextjs", () => ({
metrics: {
count: vi.fn(),
distribution: vi.fn(),
},
}));
describe("CalendarSyncService", () => {Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/__tests__/CalendarSyncService.test.ts
Lines: 118-118
Issue Type: robustness-low
Severity: low
Issue Description:
The test file does not mock `@sentry/nextjs`, but `CalendarSyncService` calls `metrics.count` and `metrics.distribution` on every code path. Every sibling test in this package (`CalendarSubscriptionService.test.ts`) explicitly mocks `@sentry/nextjs`. Without the mock the tests either fail (if Sentry throws in the test environment) or silently call through to the real SDK (potentially making network requests or causing test flakes). Add a `vi.mock("@sentry/nextjs", ...)` at the top of the file, matching the pattern used in the sibling test.
Current Code:
describe("CalendarSyncService", () => {
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| try { | ||
| await handleCancelBooking({ | ||
| userId: booking.userId, | ||
| bookingData: { | ||
| uid: booking.uid, | ||
| cancellationReason: "Cancelled on user's calendar", | ||
| cancelledBy: booking.userPrimaryEmail, | ||
| // Skip calendar event deletion to avoid infinite loops | ||
| // (Google/Office365 → Cal.com → Google/Office365 → ...) | ||
| skipCalendarSyncTaskCancellation: true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
handleCancelBooking is called without actionSource, so every calendar-sync-triggered cancellation defaults to "UNKNOWN" and emits a log.warn("Booking cancellation with unknown actionSource", ...) (handleCancelBooking.ts:181-188). This produces permanent log noise for a legitimate, expected code path. The ActionSourceSchema enum (packages/features/booking-audit/lib/types/actionSource.ts) does not include a "CALENDAR_SYNC" value; "WEBHOOK" is the closest semantic match and would suppress the spurious warning.
Suggested fix
await handleCancelBooking({
userId: booking.userId,
actionSource: "WEBHOOK",
bookingData: {
uid: booking.uid,
cancellationReason: "Cancelled on user's calendar",
cancelledBy: booking.userPrimaryEmail,
// Skip calendar event deletion to avoid infinite loops
// (Google/Office365 → Cal.com → Google/Office365 → ...)
skipCalendarSyncTaskCancellation: true,
},
});Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts
Lines: 104-115
Issue Type: maintainability-medium
Severity: medium
Issue Description:
`handleCancelBooking` is called without `actionSource`, so every calendar-sync-triggered cancellation defaults to `"UNKNOWN"` and emits a `log.warn("Booking cancellation with unknown actionSource", ...)` (handleCancelBooking.ts:181-188). This produces permanent log noise for a legitimate, expected code path. The `ActionSourceSchema` enum (`packages/features/booking-audit/lib/types/actionSource.ts`) does not include a `"CALENDAR_SYNC"` value; `"WEBHOOK"` is the closest semantic match and would suppress the spurious warning.
Current Code:
await handleCancelBooking({
userId: booking.userId,
bookingData: {
uid: booking.uid,
cancellationReason: "Cancelled on user's calendar",
cancelledBy: booking.userPrimaryEmail,
// Skip calendar event deletion to avoid infinite loops
// (Google/Office365 → Cal.com → Google/Office365 → ...)
skipCalendarSyncTaskCancellation: true,
},
});
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| eventTypeId: booking.eventTypeId, | ||
| start: event.start?.toISOString() ?? booking.startTime.toISOString(), | ||
| end: event.end?.toISOString() ?? booking.endTime.toISOString(), | ||
| timeZone: event.timeZone ?? "UTC", |
There was a problem hiding this comment.
language: "en" is hardcoded in the rescheduleBooking call to createBooking. The booking object already contains user locale information accessible via the organizer's profile. Using a hardcoded "en" means all reschedule notifications triggered via calendar sync will be sent in English regardless of the organizer's and attendees' configured locale, breaking the i18n experience.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts
Lines: 176-176
Issue Type: functional-medium
Severity: medium
Issue Description:
`language: "en"` is hardcoded in the `rescheduleBooking` call to `createBooking`. The booking object already contains user locale information accessible via the organizer's profile. Using a hardcoded `"en"` means all reschedule notifications triggered via calendar sync will be sent in English regardless of the organizer's and attendees' configured locale, breaking the i18n experience.
Current Code:
language: "en",
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| } catch (error) { | ||
| // Log error but don't block - calendar change should still be reflected | ||
| log.error("Failed to cancel booking from calendar sync", { bookingUid, error: safeStringify(error) }); |
There was a problem hiding this comment.
When handleCancelBooking throws for a legitimate business-logic reason: e.g. eventType.disableCancelling === true (HTTP 400), or the booking has already ended (HTTP 400): the error is caught, logged, and silently discarded. The result is a persistent split-brain state: the external calendar (Google/Office365) shows the event as cancelled, but the cal.com booking remains CONFIRMED in the database. There is no retry, alert, or dead-letter queue. Consider distinguishing between transient errors (worth retrying) and permanent business-rule rejections (worth surfacing via a metric or separate alert rather than simply swallowing).
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts
Lines: 124-126
Issue Type: robustness-medium
Severity: medium
Issue Description:
When `handleCancelBooking` throws for a legitimate business-logic reason: e.g. `eventType.disableCancelling === true` (HTTP 400), or the booking has already ended (HTTP 400): the error is caught, logged, and silently discarded. The result is a persistent split-brain state: the external calendar (Google/Office365) shows the event as cancelled, but the cal.com booking remains `CONFIRMED` in the database. There is no retry, alert, or dead-letter queue. Consider distinguishing between transient errors (worth retrying) and permanent business-rule rejections (worth surfacing via a metric or separate alert rather than simply swallowing).
Current Code:
// Log error but don't block - calendar change should still be reflected
log.error("Failed to cancel booking from calendar sync", { bookingUid, error: safeStringify(error) });
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Security Scan Summary
No critical security issues detected Scan completed in 12.8sSecurity scan powered by Codity.ai |
License Compliance Scan
Weak copyleft licenses found - verify compatibility Some packages have unknown licenses - manual review required Medium Risk Licenses - 5 packages(MPL-2.0 OR Apache-2.0) (1 packages):
MPL-2.0 (4 packages):
Unknown Licenses - 44 packages
...and 24 more Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/cal.com · PR #6Scanned: 2026-05-31 09:16 UTC | Score: 52/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-001]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
packages/features/bookings/lib/handleCancelBooking.ts |
0 | 1 | 1 | 0 | 2 |
packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts |
0 | 0 | 1 | 1 | 2 |
packages/features/calendar-subscription/lib/sync/__tests__/CalendarSyncService.test.ts |
0 | 0 | 1 | 14 | 15 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
3 participants
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist