Async import of the appStore packages

[zaibkhan]

No description provided.

[codoki-pr-intelligence]

Codoki PR Review

Summary: Guard async app imports, prevent crashes
What’s good: Switching to awaited appStore access aligns with async imports and prevents using unresolved modules.
Review Status: ❌ Requires changes
Overall Priority: High
Review Update:
• Coverage: Reviewed all 12 files across 2 batches

Issues (Critical & High only)

Severity	Issue	Why it matters
High	Correctness — Async forEach causes unawaited deletions and swallowed er...	…/lib/reschedule.ts Async forEach is not awaited, so calendar/video deletions run in the background and errors won't be caught by the surrounding try/catch. This can leave orphaned events or miss deletions under load.
High	Correctness — Unawaited async forEach in reschedule leads to inconsiste...	…/lib/reschedule.ts Same issue: async forEach is not awaited, so deleteEvent/deleteMeeting operations can be dropped or finish after the handler completes, and exceptions won't be caught.
High	Correctness — Incorrect last attendee check (length < 0 is impossible)	…/lib/handleCancelBooking.ts The subsequent comparison uses length < 0, which is always false; when the leaving attendee is the last remaining one, this condition will not trigger deletion paths and will incorrectly attempt to update instead. Example: if all other attendees are filtered out, length is 0, but 0 < 0 is false.
High	Correctness — Unhandled rejection on async app load can crash deletePay...	…/payment/deletePayment.ts If the async appStore loader rejects (missing package, load error), this await will throw and bypass your existing guard, causing an unhandled error in a critical payment flow. Wrap the load in try/catch and return false to preserve the function's established error-handling model.
High	Correctness — Unhandled rejection on async app load can crash handlePay...	…/payment/handlePayment.ts A rejected async appStore load here will throw and skip your not-implemented guard, potentially surfacing as a 500 during booking payments. Catch loader failures and return null to align with the function's current fallback behavior.

Showing top 5 issues. Critical: 0, High: 5. See inline suggestions for more.

Key Feedback (click to expand)

• Needs improvement: Handle rejected dynamic imports so deletePayment/handlePayment follow existing guard behavior instead of throwing.
• Testing: Add tests that simulate an appStore entry rejecting to verify deletePayment returns false and handlePayment returns null (and logs), and that afterPayment is not called.
• Documentation: Document that appStore entries are now async and clarify expected behavior on failed loads (return values, logging).
• Compatibility: No observed compatibility concerns with Node 18/TypeScript for awaiting dynamic module entries inside async functions.
• Performance: If appStore loaders are not memoized, repeated awaits could introduce cold-start latency per request; ensure appStore caches loaded modules.
• Security: No new security surfaces introduced by the await change; ensure logs do not expose sensitive credential content when handling loader errors.
• Open questions: Do we expect appStore loaders to ever reject (e.g., missing package, network)? If so, should we also emit structured error metrics for observability?

Confidence: 2/5 — Not ready to merge (5 high · status: Requires changes)

Sequence Diagram

sequenceDiagram
    participant Caller
    participant handlePayment
    participant appStore
    participant PaymentService
    participant paymentInstance
    Caller->>handlePayment: handlePayment(evt, selectedEventType, creds, booking)
    handlePayment->>appStore: [dirName]
    alt app not implemented
        handlePayment-->>Caller: return null
    else app implemented
        handlePayment->>PaymentService: new PaymentService(creds)
        PaymentService-->>handlePayment: instance
        handlePayment->>paymentInstance: create({ amount, currency }, booking.id)
        paymentInstance-->>handlePayment: paymentData
        alt paymentData falsy
            handlePayment-->>Caller: throw Error("Payment data is null")
        else paymentData truthy
            handlePayment->>paymentInstance: afterPayment(evt, booking, paymentData)
            paymentInstance-->>handlePayment: void
            handlePayment-->>Caller: paymentData
        end
    end

Loading

React with 👍 or 👎 if you found this review useful.

[codoki-pr-intelligence]

⚠️ High: Async forEach is not awaited, so calendar/video deletions run in the background and errors won't be caught by the surrounding try/catch. This can leave orphaned events or miss deletions under load.

Suggested change

	bookingRefsFiltered.forEach(async (bookingRef) => {
	```suggestion
	const tasks = bookingRefsFiltered.map(async (bookingRef) => {
	if (!bookingRef.uid) return;
	if (bookingRef.type.endsWith("_calendar")) {
	const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
	return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
	}
	if (bookingRef.type.endsWith("_video")) {
	return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);
	}
	});
	await Promise.all(tasks);

[codoki-pr-intelligence]

⚠️ High: Same issue: async forEach is not awaited, so deleteEvent/deleteMeeting operations can be dropped or finish after the handler completes, and exceptions won't be caught.

Suggested change

	bookingRefsFiltered.forEach(async (bookingRef) => {
	```suggestion
	const tasks = bookingRefsFiltered.map(async (bookingRef) => {
	if (!bookingRef.uid) return;
	if (bookingRef.type.endsWith("_calendar")) {
	const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
	return calendar?.deleteEvent(
	bookingRef.uid,
	builder.calendarEvent
	);
	}
	if (bookingRef.type.endsWith("_video")) {
	return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);
	}
	});
	await Promise.all(tasks);

[codoki-pr-intelligence]

⚠️ High: If the async appStore loader rejects (missing package, load error), this await will throw and bypass your existing guard, causing an unhandled error in a critical payment flow. Wrap the load in try/catch and return false to preserve the function's established error-handling model.

Suggested change

	const paymentApp = await appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
	```suggestion
	let paymentApp;
	try {
	paymentApp = await appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
	} catch (err) {
	console.warn(`Failed to load payment app: ${paymentAppCredentials?.app?.dirName}`, err);
	return false;
	}

[codoki-pr-intelligence]

⚠️ High: A rejected async appStore load here will throw and skip your not-implemented guard, potentially surfacing as a 500 during booking payments. Catch loader failures and return null to align with the function's current fallback behavior.

Suggested change

	const paymentApp = await appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
	```suggestion
	let paymentApp;
	try {
	paymentApp = await appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
	} catch (err) {
	console.warn(`Failed to load payment app: ${paymentAppCredentials?.app?.dirName}`, err);
	return null;
	}