Async import of the appStore packages

[everettbu]

Test 2

[greptile-apps]

Greptile Summary

This PR implements a significant architectural change to make all app store package imports asynchronous across the Cal.com codebase. The core change is in packages/app-store/index.ts, which converts all static imports to dynamic imports using import() statements for calendar integrations, video providers, payment processors, and other app store packages.

This refactoring touches multiple critical components:

• Calendar Management: CalendarManager.ts, EventManager.ts, and getCalendar.ts now handle async calendar loading
• Video Services: videoClient.ts converts from synchronous to async video adapter loading
• Payment Processing: Payment-related files like handlePayment.ts and deletePayment.ts now await app store access
• Booking Operations: Core booking flows in handleNewBooking.ts, handleCancelBooking.ts, and reschedule operations now handle async calendar operations

The change enables lazy loading of integrations, potentially reducing initial bundle size and improving startup performance by loading only the integrations actually needed at runtime. This is particularly beneficial for Cal.com given its extensive integration ecosystem.

PR Description Notes:

• The PR body only contains "Test 2" which lacks any meaningful description of the changes

Confidence score: 2/5

• This PR has critical implementation flaws that will cause production issues, particularly with async forEach loops that won't properly wait for operations to complete
• Multiple files use forEach with async callbacks which creates race conditions and potential data loss during booking operations
• Files with critical issues: packages/app-store/wipemycalother/lib/reschedule.ts, packages/app-store/vital/lib/reschedule.ts, packages/trpc/server/routers/viewer/bookings.tsx
• The core architectural change is sound but the implementation has serious async handling problems

_{12 files reviewed, no comments}

_{Edit Code Review Bot Settings | Greptile}

[greptile-apps]

style: Consider adding try-catch around the await to handle import failures gracefully

[greptile-apps]

style: Consider adding try-catch around the await to handle import failures gracefully

[github-actions]

This PR is being marked as stale due to inactivity.

[GitHoobar]

Review Summary

🏷️ Draft Comments (15)

Skipped posting 15 draft comments that were valid but scored below your review threshold (>=13/15). Feel free to update them here.

packages/app-store/wipemycalother/lib/reschedule.ts (1)

125-134: forEach with an async callback does not await the asynchronous operations, so calendar/video deletions may not complete before proceeding, leading to inconsistent state or missed deletions.

📊 Impact Scores:

• Production Impact: 4/5
• Fix Specificity: 5/5
• Urgency Impact: 3/5
• Total Score: 12/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In packages/app-store/wipemycalother/lib/reschedule.ts, lines 125-134, the code uses `forEach` with an async callback to delete calendar/video events, but this does not await the asynchronous operations, causing possible missed deletions or inconsistent state. Replace the `forEach` with `await Promise.all([...])` and use `map` to ensure all deletions are awaited before proceeding. Update the code so all async deletions are properly awaited.

---

packages/core/CalendarManager.ts (2)

44-113: The function getConnectedCalendars uses Promise.all over a potentially large array, but each iteration may perform multiple sequential awaits, causing cumulative latency for each credential.

📊 Impact Scores:

• Production Impact: 3/5
• Fix Specificity: 2/5
• Urgency Impact: 2/5
• Total Score: 7/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In packages/core/CalendarManager.ts, lines 44-113, the function `getConnectedCalendars` uses `Promise.all` over a potentially large array, but each iteration may perform multiple sequential awaits (e.g., `await item.calendar`, `await calendar.listCalendars()`). Review the logic to ensure that all possible async operations are parallelized where possible, and consider batching or limiting concurrency if the number of credentials is very large to avoid resource exhaustion. If possible, refactor to minimize sequential awaits within each iteration.

---

190-190: The function getNextCache uses console.log, which can cause significant performance degradation and log noise in production environments, especially if called frequently.

📊 Impact Scores:

• Production Impact: 3/5
• Fix Specificity: 5/5
• Urgency Impact: 2/5
• Total Score: 10/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In packages/core/CalendarManager.ts, line 190, replace the `console.log` statement with a proper logger (e.g., `log.debug`) to avoid performance degradation and log noise in production. This is especially important if `getNextCache` is called frequently.

---

packages/core/EventManager.ts (1)

464-515: updateAllCalendarEvents appends async functions to result array and then calls Promise.all(result), but some elements may not be promises (e.g., from earlier push), causing unpredictable concurrency and possible resource contention for large credential sets.

📊 Impact Scores:

• Production Impact: 3/5
• Fix Specificity: 4/5
• Urgency Impact: 2/5
• Total Score: 9/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In packages/core/EventManager.ts, lines 464-515, the `result` array is built by pushing both direct function results and async functions, then passed to `Promise.all(result)`. This can cause unpredictable concurrency and resource contention, especially with large credential sets. Refactor so that all elements in the array passed to `Promise.all` are guaranteed to be promises, and avoid mixing sync and async results. Use `.find` instead of `.filter(...)[0]` for single credential lookups.

---

packages/core/videoClient.ts (1)

21-35: getVideoAdapters now uses a for-loop with sequential await on each credential, causing N sequential dynamic imports and blocking on each; this can cause major latency when processing many credentials.

📊 Impact Scores:

• Production Impact: 3/5
• Fix Specificity: 5/5
• Urgency Impact: 2/5
• Total Score: 10/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In packages/core/videoClient.ts, lines 21-35, the function `getVideoAdapters` currently uses a for-loop with sequential `await` for each credential, causing N sequential dynamic imports and major latency when processing many credentials. Refactor this function to perform all dynamic imports in parallel using `Promise.all` and `Array.map`, so that all adapters are loaded concurrently. Preserve the original logic and return type.

---

packages/features/bookings/lib/handleCancelBooking.ts (4)

460-470: .forEach(async ...) is used for async operations (lines 460-470), causing unawaited Promises and potential race conditions or incomplete deletions for large datasets.

📊 Impact Scores:

• Production Impact: 4/5
• Fix Specificity: 5/5
• Urgency Impact: 3/5
• Total Score: 12/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In packages/features/bookings/lib/handleCancelBooking.ts, lines 460-470, replace the `.forEach(async (credential) => { ... })` loop with a `for...of` loop that properly awaits all async operations. This prevents unawaited Promises and ensures all calendar deletions complete before proceeding. Refactor as shown in the suggestion.

---

106-658: Large function handler (lines 106-658) is over 500 lines, making it extremely difficult to maintain, test, and optimize, increasing risk of performance and logic errors.

📊 Impact Scores:

• Production Impact: 2/5
• Fix Specificity: 3/5
• Urgency Impact: 2/5
• Total Score: 7/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In packages/features/bookings/lib/handleCancelBooking.ts, lines 106-658, the `handler` function is extremely large and complex (over 500 lines). Refactor it into smaller, well-named helper functions (e.g., attendee processing, integration deletions, payment handling, webhook dispatch) to improve maintainability, testability, and performance. Each helper should encapsulate a distinct responsibility.

---

107-412: cancellationReason from req.body is used directly in database updates and event payloads without sanitization, allowing attackers to inject malicious content (e.g., XSS payloads) that could be rendered in emails, webhooks, or logs.

📊 Impact Scores:

• Production Impact: 4/5
• Fix Specificity: 2/5
• Urgency Impact: 3/5
• Total Score: 9/15

🤖 AI Agent Prompt (Copy & Paste Ready):

Sanitize the `cancellationReason` field in packages/features/bookings/lib/handleCancelBooking.ts (lines 107-412). The value is taken directly from user input and used in database updates and event payloads, which could allow XSS or injection attacks if malicious content is submitted. Add a simple sanitization function (e.g., stripping `<` and `>`) and ensure all uses of `cancellationReason` use the sanitized value.

---

327-327, 591-591: console.error and console.warn leak potentially sensitive error details to logs, which may expose internal information or user data if logs are accessible to unauthorized parties.

📊 Impact Scores:

• Production Impact: 2/5
• Fix Specificity: 2/5
• Urgency Impact: 2/5
• Total Score: 6/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In packages/features/bookings/lib/handleCancelBooking.ts, lines 327 and 591, remove or replace `console.error` and `console.warn` statements with secure logging mechanisms that do not leak sensitive information. These statements may expose internal errors or user data in logs accessible to unauthorized parties.

---

packages/features/bookings/lib/handleNewBooking.ts (3)

312-312: console.log statements in production code (e.g., line 312) can cause significant performance degradation under high load and pollute logs; use structured logging instead.

📊 Impact Scores:

• Production Impact: 3/5
• Fix Specificity: 5/5
• Urgency Impact: 2/5
• Total Score: 10/15

🤖 AI Agent Prompt (Copy & Paste Ready):

Replace the `console.log` statement at line 312 in packages/features/bookings/lib/handleNewBooking.ts with a proper logger call. Use `log.debug` or an equivalent structured logging method to avoid performance and maintainability issues in production.

---

639-639: Use of Object.prototype.hasOwnProperty directly on objects (lines 639, 644) can cause major performance and correctness issues if the object does not inherit from Object; use Object.hasOwn or a safe check.

📊 Impact Scores:

• Production Impact: 2/5
• Fix Specificity: 5/5
• Urgency Impact: 2/5
• Total Score: 9/15

🤖 AI Agent Prompt (Copy & Paste Ready):

Replace the direct use of `hasOwnProperty` on `eventType` at line 639 in packages/features/bookings/lib/handleNewBooking.ts with `Object.hasOwn(eventType, "bookingLimits")` to avoid prototype pollution and performance issues.

---

404-439: req.body is parsed and used for booking creation without explicit sanitization, risking injection attacks (e.g., prototype pollution or malicious payloads) if upstream validation is bypassed.

📊 Impact Scores:

• Production Impact: 3/5
• Fix Specificity: 2/5
• Urgency Impact: 2/5
• Total Score: 7/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In packages/features/bookings/lib/handleNewBooking.ts, lines 404-439, user input from `req.body` is parsed and used for booking creation without explicit sanitization, which could allow prototype pollution or injection attacks if upstream validation is bypassed. Add a deep sanitization function after parsing to remove dangerous keys like `__proto__`, `constructor`, and `prototype` from all objects in `reqBody` before further processing. Ensure this is done before any use of `reqBody`.

---

packages/lib/payment/deletePayment.ts (1)

18-18: console.warn in production code can cause significant performance degradation under high load due to synchronous I/O and should be replaced with proper logging or removed.

📊 Impact Scores:

• Production Impact: 3/5
• Fix Specificity: 4/5
• Urgency Impact: 2/5
• Total Score: 9/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In packages/lib/payment/deletePayment.ts, line 18, there is a `console.warn` statement used for error reporting. Console statements in production can cause synchronous I/O and degrade performance under high load. Please remove this statement or replace it with a non-blocking, production-grade logger. If logging is not required, comment it out or delete it.

---

packages/trpc/server/routers/viewer/bookings.tsx (2)

553-567: forEach callback is async but not awaited, so calendar/video deletions may not complete before proceeding, leading to race conditions and missed deletions.

📊 Impact Scores:

• Production Impact: 4/5
• Fix Specificity: 5/5
• Urgency Impact: 3/5
• Total Score: 12/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In packages/trpc/server/routers/viewer/bookings.tsx, lines 553-567, the code uses `forEach` with an async callback to delete calendar/video events, but does not await the deletions, causing race conditions and possible missed deletions. Replace the `forEach(async ...)` with a `for...of` loop and `await` each deletion to ensure all deletions complete before proceeding.

---

967-967: appStore property access is now awaited (e.g., await appStore[...]), but if appStore is not a trusted, validated object, this could allow prototype pollution or arbitrary code execution if an attacker controls appStore keys.

📊 Impact Scores:

• Production Impact: 4/5
• Fix Specificity: 3/5
• Urgency Impact: 4/5
• Total Score: 11/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In packages/trpc/server/routers/viewer/bookings.tsx, lines 967-967, the code dynamically accesses and awaits a property on `appStore` using a value from the database (`paymentAppCredential?.app?.dirName`). This could allow prototype pollution or arbitrary code execution if an attacker controls the dirName. Add a whitelist check to ensure only valid, expected keys are used for property access before awaiting. Reject or throw if the dirName is not in the allowed list.

---

🔍 Comments beyond diff scope (2)

packages/core/CalendarManager.ts (1)

184-199: getNextCache fetches and parses JSON from a URL containing user-controlled username and month parameters without sanitization, enabling a reflected file path or SSRF attack if an attacker controls these values.
Category: security

---

packages/features/bookings/lib/handleCancelBooking.ts (1)

224-225: lastAttendee logic is incorrect: it checks for < 0 instead of === 0, so the code never marks the last attendee, breaking seat cancellation logic.
Category: correctness

---