Open
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
|
|
Contributor
📝 WalkthroughWalkthroughThe changes modify the Google Calendar authentication and testing modules. In CalendarAuth.ts, imports are reordered, a custom retry configuration is introduced with helper functions to determine retry eligibility based on HTTP methods and status codes, and the retry config is added to Google Calendar client instances. In the test file, helper functions receive explicit return types, assertions are broadened, and new test helpers and fixtures are added to verify retry behavior for rate-limited responses with specific status codes and error reasons. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/app-store/googlecalendar/lib/__tests__/CalendarService.auth.test.ts (1)
251-316: Optional: extract shared setup for retry tests.The three new retry tests repeat the same Arrange steps; a small helper would reduce noise and make future retry-case additions simpler.
Refactor sketch
+async function setupRegularCalendarServiceForRetryTests() { + const regularCredential = await createCredentialForCalendarService(); + mockSuccessfulCalendarListFetch(); + const calendarService = BuildCalendarService(regularCredential); + await calendarService.listCalendars(); + return { regularCredential, calendarService }; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/googlecalendar/lib/__tests__/CalendarService.auth.test.ts` around lines 251 - 316, Extract the repeated Arrange steps in the three retry tests into a single helper (e.g., createRetryTestContext) that performs createCredentialForCalendarService(), mockSuccessfulCalendarListFetch(), instantiates BuildCalendarService(credential) and calls calendarService.listCalendars(), and returns whatever callers need (the calendarService and getCalendarRetryConfig() result or shouldRetry). Replace the duplicate setup in the tests with calls to this helper and use its returned retryConfig/shouldRetry in the assertions to reduce duplication and make future retry tests simpler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app-store/googlecalendar/lib/CalendarAuth.ts`:
- Around line 355-358: The callback path instantiates the Google Calendar client
without the retry config; update the calendar_v3.Calendar construction in
callback.ts (where new calendar_v3.Calendar({ auth: oAuth2Client }) is created)
to include retryConfig: GOOGLE_CALENDAR_RETRY_CONFIG so it matches
CalendarAuth.ts usage and gains the same retry behavior; ensure the symbol
GOOGLE_CALENDAR_RETRY_CONFIG is imported/available in callback.ts before adding
it.
---
Nitpick comments:
In
`@packages/app-store/googlecalendar/lib/__tests__/CalendarService.auth.test.ts`:
- Around line 251-316: Extract the repeated Arrange steps in the three retry
tests into a single helper (e.g., createRetryTestContext) that performs
createCredentialForCalendarService(), mockSuccessfulCalendarListFetch(),
instantiates BuildCalendarService(credential) and calls
calendarService.listCalendars(), and returns whatever callers need (the
calendarService and getCalendarRetryConfig() result or shouldRetry). Replace the
duplicate setup in the tests with calls to this helper and use its returned
retryConfig/shouldRetry in the assertions to reduce duplication and make future
retry tests simpler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc605b65-b329-4490-98b6-983e1e7bad36
📒 Files selected for processing (2)
packages/app-store/googlecalendar/lib/CalendarAuth.tspackages/app-store/googlecalendar/lib/__tests__/CalendarService.auth.test.ts
Comment on lines
355
to
358
| return new calendar_v3.Calendar({ | ||
| auth: googleAuthClient, | ||
| retryConfig: GOOGLE_CALENDAR_RETRY_CONFIG, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect all Calendar client instantiations and nearby arguments
rg -n -C4 'new\s+calendar_v3\.Calendar\s*\(' packages/app-store/googlecalendar
# Inspect where retryConfig is actually supplied
rg -n -C2 'retryConfig\s*:' packages/app-store/googlecalendarRepository: calcom/cal.com
Length of output: 3988
Add retryConfig to the Calendar client instantiation in callback.ts.
The gap is real. While CalendarAuth.ts:355 correctly passes retryConfig: GOOGLE_CALENDAR_RETRY_CONFIG, the constructor at callback.ts:89 instantiates new calendar_v3.Calendar({ auth: oAuth2Client }) without it. This inconsistency leaves the callback path unprotected if it later performs write operations or is extended to do so.
Apply the same retryConfig: GOOGLE_CALENDAR_RETRY_CONFIG to the callback.ts constructor for consistency and resilience.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/app-store/googlecalendar/lib/CalendarAuth.ts` around lines 355 -
358, The callback path instantiates the Google Calendar client without the retry
config; update the calendar_v3.Calendar construction in callback.ts (where new
calendar_v3.Calendar({ auth: oAuth2Client }) is created) to include retryConfig:
GOOGLE_CALENDAR_RETRY_CONFIG so it matches CalendarAuth.ts usage and gains the
same retry behavior; ensure the symbol GOOGLE_CALENDAR_RETRY_CONFIG is
imported/available in callback.ts before adding it.
Author
There was a problem hiding this comment.
yo thanks for this. i checked the callback path, but it is only used during OAuth setup to read the primary calendar via getPrimaryCalendar(), not for the event write calls involved in #28834.
im keeping this PR scoped to the Google Calendar event write path retry behavior (insert / patch / update) and not expanding it to unrelated client construction paths.
Contributor
There was a problem hiding this comment.
@notcodesid, that's a fair scoping decision — since callback.ts only calls getPrimaryCalendar() during OAuth setup (a read), the retry config for write operations isn't applicable there. Thanks for the clarification!
(\_/)
(•ᴗ•)
/> 📋 noted!
✏️ Learnings added
Learnt from: notcodesid
Repo: calcom/cal.com PR: 28867
File: packages/app-store/googlecalendar/lib/CalendarAuth.ts:355-358
Timestamp: 2026-04-13T12:17:26.280Z
Learning: In calcom/cal.com PR `#28867`, the `callback.ts` file in `packages/app-store/googlecalendar/` instantiates `new calendar_v3.Calendar({ auth: oAuth2Client })` without a `retryConfig`. This is intentional: the callback path is only used during OAuth setup to read the primary calendar via `getPrimaryCalendar()` and does not perform any write operations (`insert`/`patch`/`update`). Do not flag the absence of `retryConfig` in `callback.ts` as a gap — it is unrelated to the Google Calendar write-path retry behavior fixed in PR `#28867`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: yuvrajangadsingh
Repo: calcom/cal.com PR: 26811
File: packages/trpc/server/routers/viewer/slots/util.ts:884-893
Timestamp: 2026-04-08T12:26:02.746Z
Learning: In calcom/cal.com PR `#26811` (feat: check guest availability during host reschedule), the `_getGuestBusyTimesForReschedule` method in `packages/trpc/server/routers/viewer/slots/util.ts` intentionally only checks Cal.com ACCEPTED/PENDING bookings (via `BookingRepository.findBookingsByUserIdsAndDateRange`) rather than full calendar availability (calendar events, schedule rules, OOO/travel). This is a deliberate MVP scoping decision by the author (yuvrajangadsingh); accessing guest calendar credentials for full availability is deferred as a follow-up. Do not flag this as a bug.
Learnt from: vijayraghav-io
Repo: calcom/cal.com PR: 16878
File: packages/app-store/larkcalendar/api/callback.ts:109-117
Timestamp: 2025-09-08T10:07:30.026Z
Learning: In office365calendar and zohocalendar callback handlers, vijayraghav-io chose to manually call BookingReferenceRepository.reconnectWithNewCredential() after credential creation instead of using SelectedCalendarRepository.upsert(), to avoid disturbing existing error handling logic in catch blocks while still ensuring booking reference reconnection happens.
Learnt from: Udit-takkar
Repo: calcom/cal.com PR: 22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.896Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Learnt from: vijayraghav-io
Repo: calcom/cal.com PR: 16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: Four calendar integrations in Cal.com still use direct prisma.selectedCalendar.create instead of SelectedCalendarRepository.create: feishucalendar, zohocalendar, office365calendar, and larkcalendar. These bypass repository hooks and won't trigger reconnection logic for BookingReferences.
Learnt from: Udit-takkar
Repo: calcom/cal.com PR: 22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR `#22919`, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Learnt from: ShashwatPS
Repo: calcom/cal.com PR: 23638
File: packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.test.ts:198-199
Timestamp: 2025-09-06T11:00:34.372Z
Learning: In calcom/cal.com PR `#23638`, the maintainer ShashwatPS determined that authorization checks in the setDestinationReminder handler are "not applicable" when CodeRabbit suggested adding user-scoped WHERE clauses to prevent users from modifying other users' destination calendar reminder settings.
Learnt from: Udit-takkar
Repo: calcom/cal.com PR: 22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/testCall.handler.ts:41-44
Timestamp: 2025-08-27T12:15:43.830Z
Learning: In calcom/cal.com, the AgentService.getAgent() method in packages/features/calAIPhone/providers/retellAI/services/AgentService.ts does NOT include authorization checks - it only validates the agentId parameter and directly calls the repository without verifying user/team access. This contrasts with other methods like getAgentWithDetails() which properly use findByIdWithUserAccessAndDetails() for authorization. When reviewing updateToolsFromAgentId() calls, always verify both agent ownership and eventType ownership are checked.
Learnt from: Udit-takkar
Repo: calcom/cal.com PR: 22995
File: packages/features/ee/workflows/lib/reminders/aiPhoneCallManager.ts:167-193
Timestamp: 2025-08-19T08:45:41.834Z
Learning: In calcom/cal.com AI phone call scheduling (aiPhoneCallManager.ts), workflow reminder records are intentionally kept in the database even when task scheduling fails, as they provide valuable debugging information for troubleshooting scheduling issues, per maintainer Udit-takkar in PR `#22995`.
Learnt from: din-prajapati
Repo: calcom/cal.com PR: 21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
Learnt from: CR
Repo: calcom/cal.com PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-06T12:40:06.867Z
Learning: Applies to apps/api/v2/**/*.{ts,tsx} : When importing from `calcom/features` or `calcom/trpc` into `apps/api/v2`, re-export from `packages/platform/libraries/index.ts` and import from `calcom/platform-libraries` instead of direct imports
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(google-calendar): retry rate-limited write requests
Fixes a Google Calendar retry gap that could leave bookings out of sync with calendar events when Google returns transient rate-limit errors.
This PR adds an explicit retry policy to the Google Calendar client so
PATCHandPOSTwrite requests can be retried, and403responses are only retried when Google marks them as rate-limit errors.Fixes #28834
How should this be tested?