fix: handle collective multiple host on destinationCalendar

[CodingKylo]

Martian Code Review Benchmark PR (mirrored from source #4)

[re-entry-ai]

🛡️ re-entry.ai Code Review

@CodingKylo — Please review the following assessment:

🚨 Risk Score: 83/100 · CRITICAL

Dimension	Level
Likelihood	🔴 Critical
Impact	🔴 Critical
Detectability	🟡 Medium

Intent

Update booking/event handling to support multiple destination calendars for collective/team bookings, and ensure calendar integrations (Google/Lark/Office365) and reminder emails use the correct destination calendar.

Summary

The PR changes CalendarEvent.destinationCalendar from a single destination calendar shape to an array, and updates some integrations/reminder cron to wrap or select a “main host” destination calendar. However, multiple call sites still persist or consume only destinationCalendar[0], which can silently drop additional destination calendars for collective bookings. There is also a contract mismatch risk: GoogleCalendarService.createEvent now takes credentialId, but the call chain and the selection logic must be aligned with the same destination calendar used for the integration. You should verify that every booking flow (create/cancel/reschedule/reminders) normalizes destinationCalendar consistently and that the integration selection uses the correct credentialId/calendar pair.

🎯 Review Focus

Verify end-to-end contract consistency for CalendarEvent.destinationCalendar (array vs scalar) across: booking creation persistence (handleNewBooking), cancellation/reschedule flows, reminder cron (bookingReminder), and each calendar integration’s selection logic (especially Google’s new createEvent(calEventRaw, credentialId) path).

Key Findings

• 🚨 [packages/app-store/googlecalendar/lib/CalendarService.ts:L84] CRITICAL: createEvent signature changed to require credentialId, but the service also derives mainHostDestinationCalendar from calEventRaw.destinationCalendar and then selects organizer email/calendar from that element. If the caller passes a credentialId that doesn’t correspond to the same destination calendar chosen as mainHostDestinationCalendar, you can create events in the wrong host calendar or with the wrong organizer identity. Fix: make the selection deterministic from the same inputs: use the passed credentialId to pick the destination calendar (and error if not found), e.g. const mainHostDestinationCalendar = calEventRaw.destinationCalendar?.find(c => c.credentialId === credentialId); if (!mainHostDestinationCalendar) throw ...; and then use only mainHostDestinationCalendar for organizer email and calendar selection.

✅ Action Checklist

• [ ] CRITICAL [packages/app-store/googlecalendar/lib/CalendarService.ts:L84] — createEvent signature changed to require credentialId, but the service also derives mainHostDestinationCalendar from calEventRaw.destinationCalendar and then selects organizer email/calendar from that element. If the caller passes a credentialId that doesn’t correspond to the same destination calendar chosen as mainHostDestinationCalendar, you can create events in the wrong host calendar or with the wrong organizer identity. Fix: make the selection deterministic from the same inputs: use the passed credentialId to pick the destination calendar (and error if not found), e.g. const mainHostDestinationCalendar = calEventRaw.destinationCalendar?.find(c => c.credentialId === credentialId); if (!mainHostDestinationCalendar) throw ...; and then use only mainHostDestinationCalendar for organizer email and calendar selection.
• [ ] SUGGESTION — Normalize at the boundary and delete ad-hoc shape handling. For example, in apps/web/pages/api/cron/bookingReminder.ts:L104-L127, replace booking.destinationCalendar || user.destinationCalendar with a normalization helper used everywhere: const destinationCalendars = normalizeDestinationCalendars(booking.destinationCalendar, user.destinationCalendar); then set destinationCalendar: destinationCalendars;. Apply the same helper in all booking flows (new/cancel/reschedule) and in all integration services.
• [ ] SUGGESTION — Fix the systemic “main host only” workaround consistently. In packages/app-store/larkcalendar/lib/CalendarService.ts:L125-L189 and packages/app-store/office365calendar/lib/CalendarService.ts:L70-L70, you do const [mainHostDestinationCalendar] = event.destinationCalendar ?? [];. If the product requirement is to create events for multiple destination calendars, these services must iterate over event.destinationCalendar and create/update/delete per destination calendar. If the requirement is truly “only one host calendar is supported for creation,” then enforce it explicitly by validating event.destinationCalendar.length <= 1 (or selecting by credentialId) and logging/throwing when multiple are provided.
• [ ] SUGGESTION — In packages/app-store/googlecalendar/lib/CalendarService.ts:L138-L142 (truncated in diff), ensure the selectedCalendar lookup is actually implemented correctly and uses the passed credentialId (not just externalId). Add a hard failure when no matching destination calendar is found for the credentialId to prevent silent misrouting.
• [ ] SUGGESTION — In packages/features/bookings/lib/handleNewBooking.ts around the isTeamEventType change, verify that eventType.schedulingType is always present for the relevant event types. The new logic !!eventType.schedulingType && ["COLLECTIVE", "ROUND_ROBIN"].includes(eventType.schedulingType) changes behavior for unexpected/undefined scheduling types; add a test covering the previous behavior to avoid regressions in collective booking detection.

Suggestions

• Normalize at the boundary and delete ad-hoc shape handling. For example, in apps/web/pages/api/cron/bookingReminder.ts:L104-L127, replace booking.destinationCalendar || user.destinationCalendar with a normalization helper used everywhere: const destinationCalendars = normalizeDestinationCalendars(booking.destinationCalendar, user.destinationCalendar); then set destinationCalendar: destinationCalendars;. Apply the same helper in all booking flows (new/cancel/reschedule) and in all integration services.
• Fix the systemic “main host only” workaround consistently. In packages/app-store/larkcalendar/lib/CalendarService.ts:L125-L189 and packages/app-store/office365calendar/lib/CalendarService.ts:L70-L70, you do const [mainHostDestinationCalendar] = event.destinationCalendar ?? [];. If the product requirement is to create events for multiple destination calendars, these services must iterate over event.destinationCalendar and create/update/delete per destination calendar. If the requirement is truly “only one host calendar is supported for creation,” then enforce it explicitly by validating event.destinationCalendar.length <= 1 (or selecting by credentialId) and logging/throwing when multiple are provided.
• In packages/app-store/googlecalendar/lib/CalendarService.ts:L138-L142 (truncated in diff), ensure the selectedCalendar lookup is actually implemented correctly and uses the passed credentialId (not just externalId). Add a hard failure when no matching destination calendar is found for the credentialId to prevent silent misrouting.
• In packages/features/bookings/lib/handleNewBooking.ts around the isTeamEventType change, verify that eventType.schedulingType is always present for the relevant event types. The new logic !!eventType.schedulingType && ["COLLECTIVE", "ROUND_ROBIN"].includes(eventType.schedulingType) changes behavior for unexpected/undefined scheduling types; add a test covering the previous behavior to avoid regressions in collective booking detection.

📝 This review includes 3 inline comments (3 warnings)

---

Posted by re-entry.ai · Risk governance for autonomous engineering teams

[re-entry-ai]

⚠️ WARNING

The new lookup only works when destinationCalendar is an array and a matching credentialId exists, but the fallback path no longer defaults to `

---

re-entry.ai

[re-entry-ai]

⚠️ WARNING

This persists only evt.destinationCalendar[0] even though the diff changes CalendarEvent.destinationCalendar to an array. For collective/team bookings with multiple destination calendars, the booking record will now lose the additional calendars and downstream cancellation/update flows will only know about the first one. The root cause is that the persistence layer still assumes a single destination calendar while the event model has been widened to many.

---

re-entry.ai

[re-entry-ai]

⚠️ WARNING

destinationCalendar is now wrapped in an array, but this endpoint previously passed a single calendar object. If sendOrganizerRequestReminderEmail or its template logic still expects a single DestinationCalendar, this changes the contract and can break calendar-name/integration rendering. The root cause is an API shape change without a corresponding consumer update.

---

re-entry.ai

[re-entry-ai]

🛡️ re-entry.ai Code Review

@CodingKylo — Please review the following assessment:

🚨 Risk Score: 83/100 · CRITICAL

Dimension	Level
Likelihood	🔴 Critical
Impact	🔴 Critical
Detectability	🟡 Medium

Intent

Update booking/event creation and calendar integrations to support multiple destination calendars per event, and fix host selection when multiple calendars are present.

Summary

The PR changes the destinationCalendar shape to support multiple destination calendars and updates several calendar services/entrypoints to select a “main host” calendar (often via array index 0). The highest risk is correctness: multiple layers now derive organizer email and calendar IDs from the first element or from a credentialId match, which can silently pick the wrong calendar if ordering/selection is inconsistent. There are also likely runtime issues around nullish calendars and credential lookup/usage that can cause missed deletions or failed updates. Reviewers must verify that all call sites pass a consistent DestinationCalendar[] payload and that every integration deterministically selects the intended primary calendar (not positional ordering).

🎯 Review Focus

Verify deterministic primary destination-calendar selection end-to-end (cron/webhook -> CalendarEvent payload -> each calendar integration) and eliminate any reliance on destinationCalendar[0] unless it is explicitly guaranteed by contract; ensure organizer email/calendarId/credential mapping all use the same selection logic.

Key Findings

• 🚨 [packages/app-store/googlecalendar/lib/CalendarService.ts:L97] CRITICAL: createEvent now takes credentialId but the code still derives organizer email and calendar selection from destinationCalendar without a guaranteed match. Excerpt: const [mainHostDestinationCalendar] = calEventRaw?.destinationCalendar && calEventRaw?.destinationCalendar.length > 0 ? calEventRaw.destinationCalendar : []; and email: mainHostDestinationCalendar?.externalId ? mainHostDestinationCalendar.externalId : calEventRaw.organizer.email, — This will silently use the first destination calendar even if it does not correspond to the provided credentialId, causing events to be created under the wrong organizer identity/calendar. Fix: deterministically select the calendar by credentialId (or an explicit isPrimary/priority field) and use that selection for both organizer email and calendarId. Example: const main = calEventRaw.destinationCalendar?.find(c => c.credentialId === credentialId) ?? calEventRaw.destinationCalendar?.[0]; and then use main for organizer email and calendar selection; if no match, throw a 4xx/5xx with actionable logging rather than falling back to index 0.
• ⚠️ [apps/web/pages/api/cron/bookingReminder.ts:L104] WARNING: Cron reminder builds destinationCalendar as an array but does not ensure it matches the intended credential/primary host. Excerpt: const selectedDestinationCalendar = booking.destinationCalendar || user.destinationCalendar; and destinationCalendar: selectedDestinationCalendar ? [selectedDestinationCalendar] : [], — If booking.destinationCalendar is already a DestinationCalendar[] (new contract) or if user.destinationCalendar is not the correct primary for the booking, this will produce a nested/incorrect shape or pick the wrong host. Fix: normalize based on the actual type and select the primary deterministically. Example: const dest = booking.destinationCalendar ?? user.destinationCalendar; const destinationCalendars = Array.isArray(dest) ? dest : dest ? [dest] : []; const primary = destinationCalendars.find(c => c.credentialId === booking.credentialId) ?? destinationCalendars[0]; evt.destinationCalendar = primary ? [primary] : []; (and ensure booking.credentialId exists in this handler).

✅ Action Checklist

• [ ] CRITICAL [packages/app-store/googlecalendar/lib/CalendarService.ts:L97] — createEvent now takes credentialId but the code still derives organizer email and calendar selection from destinationCalendar without a guaranteed match. Excerpt: const [mainHostDestinationCalendar] = calEventRaw?.destinationCalendar && calEventRaw?.destinationCalendar.length > 0 ? calEventRaw.destinationCalendar : []; and email: mainHostDestinationCalendar?.externalId ? mainHostDestinationCalendar.externalId : calEventRaw.organizer.email, — This will silently use the first destination calendar even if it does not correspond to the provided credentialId, causing events to be created under the wrong organizer identity/calendar. Fix: deterministically select the calendar by credentialId (or an explicit isPrimary/priority field) and use that selection for both organizer email and calendarId. Example: const main = calEventRaw.destinationCalendar?.find(c => c.credentialId === credentialId) ?? calEventRaw.destinationCalendar?.[0]; and then use main for organizer email and calendar selection; if no match, throw a 4xx/5xx with actionable logging rather than falling back to index 0.
• [ ] WARNING [apps/web/pages/api/cron/bookingReminder.ts:L104] — Cron reminder builds destinationCalendar as an array but does not ensure it matches the intended credential/primary host. Excerpt: const selectedDestinationCalendar = booking.destinationCalendar || user.destinationCalendar; and destinationCalendar: selectedDestinationCalendar ? [selectedDestinationCalendar] : [], — If booking.destinationCalendar is already a DestinationCalendar[] (new contract) or if user.destinationCalendar is not the correct primary for the booking, this will produce a nested/incorrect shape or pick the wrong host. Fix: normalize based on the actual type and select the primary deterministically. Example: const dest = booking.destinationCalendar ?? user.destinationCalendar; const destinationCalendars = Array.isArray(dest) ? dest : dest ? [dest] : []; const primary = destinationCalendars.find(c => c.credentialId === booking.credentialId) ?? destinationCalendars[0]; evt.destinationCalendar = primary ? [primary] : []; (and ensure booking.credentialId exists in this handler).
• [ ] SUGGESTION — Centralize destination-calendar normalization to avoid drift across handlers/services. Add a helper like normalizeDestinationCalendars(input): DestinationCalendar[] and selectPrimaryDestinationCalendar(destinationCalendars, credentialId): DestinationCalendar | undefined, then use it in all booking flows and in each calendar service. Example touchpoints: apps/web/pages/api/cron/bookingReminder.ts and packages/app-store/*/lib/CalendarService.ts (where you currently do const [mainHostDestinationCalendar] = ...).
• [ ] SUGGESTION — Remove positional semantics (destinationCalendar[0]) from all integrations. In packages/app-store/larkcalendar/lib/CalendarService.ts and packages/app-store/office365calendar/lib/CalendarService.ts, you currently do const [mainHostDestinationCalendar] = event.destinationCalendar ?? []; and then use it for calendarId. Fix by selecting by credentialId (or explicit primary marker) and throw if not found, rather than defaulting to index 0.
• [ ] SUGGESTION — In packages/app-store/googlecalendar/lib/CalendarService.ts, ensure the new credentialId parameter is actually used for selection. Right now the excerpt shows a fallback to first element and then a truncated find by credentialId. Fix by using the find result as the single source of truth for both organizer email and calendar selection; do not compute mainHostDestinationCalendar from index 0 at all.
• [ ] SUGGESTION — Add runtime shape validation at API boundaries where destinationCalendar is constructed (e.g., cron handler). Ensure you never accidentally convert DestinationCalendar[] into [DestinationCalendar[]] or otherwise nest arrays. Example: check Array.isArray(booking.destinationCalendar) before wrapping.

Suggestions

• Centralize destination-calendar normalization to avoid drift across handlers/services. Add a helper like normalizeDestinationCalendars(input): DestinationCalendar[] and selectPrimaryDestinationCalendar(destinationCalendars, credentialId): DestinationCalendar | undefined, then use it in all booking flows and in each calendar service. Example touchpoints: apps/web/pages/api/cron/bookingReminder.ts and packages/app-store/*/lib/CalendarService.ts (where you currently do const [mainHostDestinationCalendar] = ...).
• Remove positional semantics (destinationCalendar[0]) from all integrations. In packages/app-store/larkcalendar/lib/CalendarService.ts and packages/app-store/office365calendar/lib/CalendarService.ts, you currently do const [mainHostDestinationCalendar] = event.destinationCalendar ?? []; and then use it for calendarId. Fix by selecting by credentialId (or explicit primary marker) and throw if not found, rather than defaulting to index 0.
• In packages/app-store/googlecalendar/lib/CalendarService.ts, ensure the new credentialId parameter is actually used for selection. Right now the excerpt shows a fallback to first element and then a truncated find by credentialId. Fix by using the find result as the single source of truth for both organizer email and calendar selection; do not compute mainHostDestinationCalendar from index 0 at all.
• Add runtime shape validation at API boundaries where destinationCalendar is constructed (e.g., cron handler). Ensure you never accidentally convert DestinationCalendar[] into [DestinationCalendar[]] or otherwise nest arrays. Example: check Array.isArray(booking.destinationCalendar) before wrapping.

📝 This review includes 3 inline comments (3 warnings)

---

Posted by re-entry.ai · Risk governance for autonomous engineering teams

[re-entry-ai]

⚠️ WARNING

Valid concern: this code now treats destinationCalendar as an array and blindly takes the first entry as the “main host” calendar. That only works if callers always preserve host ordering; if the array is reordered or contains multiple calendars, the organizer email and selected calendar can be derived from the wrong host. The root cause is that the new array shape encodes multiple calendars but this service still relies on positional semantics instead of an explicit host/primary marker.

---

re-entry.ai

[re-entry-ai]

⚠️ WARNING

This is a real correctness issue if calendar can be nullish: calendar?.deleteEvent(...) yields undefined, and the cast to Promise<unknown> does not make it awaitable. Pushing undefined into apiDeletes will later break Promise.all/async handling or silently skip the deletion. The underlying problem is that the code assumes calendar lookup always succeeds, but the new multi-calendar flow makes that assumption less safe.

---

re-entry.ai

[re-entry-ai]

⚠️ WARNING

The reviewer’s symptom is real only if the credential lookup can fail. In this diff, credential is still obtained via a filtered lookup and then passed into updateEvent without a guard, so a missing match would propagate undefined into calendar updates. The root cause is that the new destinationCalendar/credential mapping is not validated before use, so the event manager can still attempt updates with an unresolved credential.

---

re-entry.ai