Cal.com / PR #30

feat: add calendar cache status and actions (#22532)

calcom/cal.com#22532
Golden recall: 100% Extra findings: 23

Golden Comments (2/2 found)

# Comment Severity Status FriendlyReviewer Detail
#1 The updateManyByCredentialId call uses an empty data object, which prevents Prisma's @updatedAt decorator from updating the updatedAt timestamp. This results in inaccurate cache status tracking, as the timestamp isn't updated when the cache is refreshed. To fix this, explicitly set the updatedAt field MEDIUM ✓ Found "updateManyByCredentialId throws at runtime when called with an empty data object: its only caller CalendarService.ts:1024 invokes updateManyByCredentialId(this.credential.id, {}), and Prisma rejects an empty data payload with PrismaClientValidationError, so every fetchAvailabilityAndSetCache call throws. @updatedAt is only bumped when at least one field is written, so the timestamp would not be touched anyway. Fix: pass { updatedAt: new Date() } explicitly or redesign as a touchByCredentialId" (selectedCalendar.ts L400)
#2 logic: macOS-specific sed syntax with empty string after -i flag will fail on Linux systems LOW ✓ Found "sed -i '' -E ... is BSD/macOS-only: on GNU sed (most Linux/CI) -i '' is interpreted as -i with an empty inline-script filename plus '' as the script, so the GOOGLE_WEBHOOK_URL update fails. Use a portable form (sed -i.bak + rm, or temp file + mv), and quote the variable" (test-gcal-webhooks.sh L68)

Supplementary Findings (23 findings)

Finding File Severity Legitimate?
deleteCache throws a plain new Error("Credential not found or access denied") instead of the project-standard TRPCError: the client sees a generic INTERNAL_SERVER_ERROR (HTTP 500) rather than NOT_FOUND/FORBIDDEN, so it cannot distinguish an absent credential from an ownership failure, and the UI toast fires for the wrong reason deleteCache.handler.ts:25 MEDIUM ✓ Real — error semantics leak into the UI toast; TRPCError is the repo convention
Cache deletion calls prisma.calendarCache.deleteMany(...) directly, bypassing the calendar-cache repository abstraction and the CalendarCache.init() feature-flag factory: deletion works even when the calendar-cache flag is disabled, and the mock's already-implemented deleteManyByCredential() is never used deleteCache.handler.ts:28 MEDIUM ✓ Real — feature-flag/repository abstraction bypassed on a destructive path
connectedCalendars constructs new CalendarCacheRepository() directly instead of using the CalendarCache.init() factory: with the flag disabled it still performs a real prisma.calendarCache.groupBy query on every call to a heavily-hit endpoint instead of being a no-op connectedCalendars.handler.ts:28 MEDIUM ✓ Real — hot endpoint keeps hitting the DB with the flag off
The ownership check (userId: user.id) is correct, but connectedCalendars can include delegation-credential calendars whose credentialId belongs to the delegating user: the dropdown computes hasCache purely from cacheUpdatedAt without gating on ownership, so the UI can render "Delete cached data" while the handler rejects it deleteCache.handler.ts:20 MEDIUM ✓ Real — UI/handler mismatch on delegated credentials; the intended UX must be confirmed
getCacheStatusByCredentialIds returns the max updatedAt across all rows for a credential, including rows whose expiresAt is in the past: a fully expired cache still reports a cacheUpdatedAt and the UI renders "Last updated" as if usable calendar-cache.repository.ts:174 MEDIUM ✓ Real — expired cache misreported as current; add expiresAt: { gte: new Date() } or document the semantic
deleteCacheMutation never invalidates connectedCalendars; it relies on an optional onChanged/onSuccess callback only wired in CalendarListContainer: in the EventType context the dropdown keeps showing the stale cache-status block (query uses refetchOnWindowFocus: false) until a full reload CredentialActionsDropdown.tsx:43 MEDIUM ✓ Real — stale UI until reload; mirror disconnectMutation's onSettled invalidation
The call bumps SelectedCalendar.updatedAt for every calendar under the credential on every webhook push/cache refresh, but the new cache-status feature surfaces cacheUpdatedAt from CalendarCache.updatedAt — not SelectedCalendar.updatedAt; the PR's own notes ask whether this update should be removed CalendarService.ts:1024 MEDIUM ✓ Real — dead write confirmed against the feature's own data source
In the webhook flow the service can be built from a delegation credential whose id differs from the credentialId on the SelectedCalendar rows, so updateManyByCredentialId(this.credential.id, {}) matches zero rows and updatedAt is not bumped even though the cache was refreshed CalendarService.ts:1024 MEDIUM ✓ Real — delegated path may silently no-op the update
cleanup() ends with exit 0 and is invoked on every error path (rate-limit branch, tunnel-URL extraction failure): a script that fails to obtain a tunnel URL still returns success, silently breaking CI/automation keyed on exit status test-gcal-webhooks.sh:17 MEDIUM ✓ Real — failures masked by exit 0; return non-zero on failure paths
The reuse branch relies on pgrep -f "tmole $TM_PORT" (may match stale/unrelated processes and return multiple PIDs) and re-reads a possibly missing/stale $LOG_FILE, silently falling into the "Failed to extract Tunnelmole URL" path and exiting 0 without cleanup test-gcal-webhooks.sh:28 MEDIUM ✓ Real — fragile process/URL reuse detection; fail loudly instead
The deleteCache schema is defined inline (z.object({ credentialId: z.number() })) while every other procedure in this router uses a dedicated *.schema.ts file; also z.number() should be z.number().int().positive() so fractional/negative ids are rejected early instead of silently failing the findFirst lookup _router.tsx:28 LOW ✓ Borderline: consistency nit; the schema hardening advice is sound
cacheStatusMap.get(calendar.credentialId) || null should use nullish coalescing ?? null: || works only because a Date is always truthy and is fragile if the type changes connectedCalendars.handler.ts:35 LOW ✓ Borderline: works today, fragile to type evolution
getCacheStatusByCredentialIds(credentialIds) performs no ownership (userId) scoping; safe today only because the sole caller passes user-scoped ids — the public repository API should document the "caller must pre-scope by userId" contract calendar-cache.repository.ts:173 LOW ✓ Borderline: latent, documented-contract ask rather than a current bug
A pre-existing TODO in getCachedAvailability states updatedAt "doesn't exist yet in CalendarCache table"; this MR adds exactly that column, so the TODO is now stale and should be resolved calendar-cache.repository.ts:1 LOW ✓ Real — the MR resolves the TODO and leaves it behind
The auto-generated Prisma warning header still says the column was added "without a default value ... not possible if the table is not empty", but the actual statement adds it NOT NULL DEFAULT NOW() — the warning text is now contradictory/misleading migration.sql:4 LOW ✓ Borderline: cosmetic, but a misleading auto-generated comment
Copy-paste bug: the disconnect dialog's onConfirm calls setDeleteModalOpen(false) instead of setDisconnectModalOpen(false); it still closes via DialogClose but leaves the disconnect modal state unmanaged CredentialActionsDropdown.tsx:150 LOW ✓ Real — copy-paste bug that works by accident today
The timestamp is hardcoded to new Intl.DateTimeFormat("en-US", ...), disregarding the user's active locale even though the component already calls useLocale(); the interpolation: { escapeValue: false } is also unnecessary for a pre-formatted string CredentialActionsDropdown.tsx:89 LOW ✓ Real — locale hardcoding in an i18n-aware component
Both confirmation dialogs close synchronously in onConfirm and don't wire isPending/loadingText: if the mutation fails the user gets only a toast, and rapid double-triggering is possible CredentialActionsDropdown.tsx:135 LOW ✓ Borderline: UX robustness, not a defect
The read-only cache-status header ("Cache Status" + "Last updated") is rendered inside a focusable/interactive DropdownMenuItem, giving it an erroneous affordance (focus ring, hover highlight, closes menu on activation) CredentialActionsDropdown.tsx:84 LOW ✓ Borderline: wrong affordance on a read-only block
No set -euo pipefail: unset variables (OWNED_PID, TMOLE_PID) are silently treated as empty and pipeline errors don't fail test-gcal-webhooks.sh:1 LOW ✓ Real — silent unset-variable behavior in a test script
The wait loop only greps for the exact "limited to 10 tunnels per hour" string; any other failure (port in use, tmole not installed, network/auth error) just times out and reports "Failed to extract URL" test-gcal-webhooks.sh:46 LOW ✓ Real — failure modes collapse into one misleading message
ENV_FILE="../.env" is relative to the CWD and tmole $TM_PORT is unquoted; both should be resolved/quoted relative to the script ($(dirname "$0")/../.env, tmole "$TM_PORT") test-gcal-webhooks.sh:5 LOW ✓ Real — CWD-dependent script plus unquoted args
The dev:cron script was changed to npx tsx cron-tester.ts, but tsx is not declared anywhere in the monorepo: npx tsx fetches an unpinned version at first run, bypassing the lockfile, and diverges from every other TS script which uses the declared ts-node; the edit is also unrelated to the MR's stated feature package.json:11 LOW ✓ Real — unpinned dependency fetch plus out-of-scope edit

Honest Analysis

FriendlyReviewer found both golden comments of this PR — recall 100%. Golden #1 (updateManyByCredentialId called with an empty data object) is found as the review's single red item, and the review goes beyond the reference comment: it confirms the runtime failure mode (PrismaClientValidationError on every fetchAvailabilityAndSetCache call through CalendarService.ts:1024), explains why @updatedAt would not be bumped anyway, proposes two fixes (pass { updatedAt: new Date() } or redesign as touchByCredentialId), and adds a diagnosis the golden does not mention — the method types data as SelectedCalendarUpdateInput when updateMany requires UpdateManyMutationInput, so relation-nested writes would type-check yet fail at runtime. Golden #2 (BSD-only sed -i '' in the webhook test script) is found at yellow with the correct GNU-sed failure mode.

The 23 extra findings include several substantive items: plain Error instead of TRPCError (generic HTTP 500 masking NOT_FOUND/FORBIDDEN), the repository/feature-flag bypass in both deleteCache.handler.ts and connectedCalendars.handler.ts (real DB work even with the calendar-cache flag disabled, and the mock's implemented deleteManyByCredential() never used), the UI/handler mismatch on delegated credentials, expired cache rows reported as current, and the missing connectedCalendars invalidation leaving stale UI state. The review's verdict — REQUEST_CHANGES, driven by the confirmed runtime crash — is the right call.

The honest caveat is signal-to-noise: 13 of the 23 findings are blue nits — shell hygiene (set -euo pipefail, quoting, ENV_FILE resolution), an a11y affordance on a read-only menu item, a hardcoded en-US locale, missing isPending wiring, a stale TODO and a contradictory auto-generated migration comment. Several script findings (cleanup exit 0, pgrep fragility, the single-string wait loop) are real but target a developer-only test script. The review earns its verdict with the first half of its list; the second half is breadth. For a consumer, the practical takeaway is that the confirmed crash must be fixed before merge, and the rest is cleanup.

FriendlyReviewer's full review

As posted on the PR — the complete output from FriendlyReviewer.

friendly-reviewer bot commented now

Hi there 👋

🌥️ Tech 🌥️ Feat
1 high, 11 medium, 13 low 1 ok, 1 issue, 4 mitigated

Executive Summary

The MR introduces a calendar-cache management feature (view cache status, delete cached data) spanning the tRPC layer, repository layer, and a new CredentialActionsDropdown UI component. The overall architecture is sound: the authorization check in deleteCache correctly ensures the credential exists and belongs to the authenticated user, the connectedCalendars enrichment is O(n)-mapped, the migration is functionally correct, and the UI conditional-rendering (dropdown only for Google Calendar credentials with cache) and i18n keys are well done.

However, several issues block merge:

  • Confirmed runtime crash in the Google webhook flow: SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {}) passes an empty data object, which Prisma rejects at runtime (PrismaClientValidationError). Every fetchAvailabilityAndSetCache call will throw. The method also uses the wrong Prisma data type (UpdateInput vs UpdateManyMutationInput).
  • tRPC error-handling deviation: deleteCache throws a plain Error instead of TRPCError, surfacing as a generic HTTP 500 instead of NOT_FOUND/FORBIDDEN, and masking the real reason in the UI toast.
  • Feature-flag/repository abstraction bypass: both deleteCache.handler.ts and connectedCalendars.handler.ts instantiate CalendarCacheRepository/prisma.calendarCache directly instead of going through the CalendarCache.init() factory, so cache deletion/status queries run real DB work even when the calendar-cache flag is disabled, and the mock's already-implemented deleteManyByCredential() is never used.
  • Stale UI state: the deleteCache mutation does not invalidate connectedCalendars, so the cache-status block keeps showing the old timestamp until a full page reload in the EventType context.
  • Semantic/accuracy concerns: getCacheStatusByCredentialIds counts expired cache rows; a dead-write concern (the feature reads CalendarCache.updatedAt, not SelectedCalendar.updatedAt) flagged in the PR's own notes is confirmed; and the delegated-credential paths may no-op or be denied inconsistently between UI and handler.
  • Script robustness: scripts/test-gcal-webhooks.sh has BSD-only sed, all failure paths exit 0, no set -euo pipefail, and fragile reuse logic. apps/web/package.json switches dev:cron to an undeclared tsx dependency.

Minor/cosmetic issues (stale migration comment, stale TODO, copy-paste bug in disconnect dialog, hardcoded en-US locale, missing pending state on confirm dialogs, informational block rendered as a menu item, || vs ??) are noted below. Review status: REQUEST_CHANGES.

These might need a close look

  • 🔴 packages/lib/server/repository/selectedCalendar.ts (L400)
    updateManyByCredentialId throws at runtime when called with an empty data object: its only caller CalendarService.ts:1024 invokes updateManyByCredentialId(this.credential.id, {}), and Prisma rejects an empty data payload with PrismaClientValidationError, so every fetchAvailabilityAndSetCache (Google webhook/availability flow) throws. Also, @updatedAt is only bumped when at least one field is written, so an empty update would not touch the timestamp anyway. Fix: pass { updatedAt: new Date() } explicitly or redesign as a touchByCredentialId. Additionally, data is typed Prisma.SelectedCalendarUpdateInput but updateMany expects SelectedCalendarUpdateManyMutationInput (relation-nested writes would type-check yet fail at runtime); tighten the signature.

Worth checking

  • 🟡 packages/trpc/server/routers/viewer/calendars/deleteCache.handler.ts (L25)
    deleteCache throws a plain new Error("Credential not found or access denied") instead of the project-standard TRPCError. This surfaces to the client as a generic INTERNAL_SERVER_ERROR (HTTP 500) rather than NOT_FOUND/FORBIDDEN, so the client cannot distinguish "credential doesn't exist" from "not owned" from a real server fault, and the UI onError toast fires for the wrong reason. Use TRPCError with NOT_FOUND for absent credentials and FORBIDDEN/UNAUTHORIZED for ownership failures.
  • 🟡 packages/trpc/server/routers/viewer/calendars/deleteCache.handler.ts (L28)
    deleteCache calls prisma.calendarCache.deleteMany(...) directly, bypassing the calendar-cache repository abstraction and the CalendarCache.init() feature-flag factory used everywhere else. The mock already implements deleteManyByCredential() (a documented no-op when the flag is off) but the real repository/interface do not declare it, so cache deletion works even when the feature is disabled and is not routed through the same abstraction that created the cache. Add deleteManyByCredential(credentialId) to the interface + real repository and call it via CalendarCache.init(null).
  • 🟡 packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts (L28)
    connectedCalendars constructs new CalendarCacheRepository() directly instead of using the CalendarCache.init() factory. When the calendar-cache flag is disabled, this still performs a real prisma.calendarCache.groupBy query on every call to a heavily-hit endpoint instead of being a no-op through CalendarCacheRepositoryMock. Use await CalendarCache.init(null) and call getCacheStatusByCredentialIds on the returned instance.
  • 🟡 packages/trpc/server/routers/viewer/calendars/deleteCache.handler.ts (L20)
    The ownership check userId: user.id is correct/secure, but connectedCalendars can include delegation-credential calendars whose credentialId belongs to the delegating user. The CredentialActionsDropdown computes hasCache purely from cacheUpdatedAt and does not gate on ownership, so for a delegated Google credential with a cache the UI can render "Delete cached data" while the handler rejects it with an ownership error. Verify the intended UX: suppress the action for delegationCredentialId or allow managed delegated deletions.
  • 🟡 packages/features/calendar-cache/calendar-cache.repository.ts (L174)
    getCacheStatusByCredentialIds returns the max updatedAt across all rows for a credential, including rows whose expiresAt is in the past, so a fully expired cache still reports a cacheUpdatedAt and the UI renders "Last updated" as if usable. If only currently-valid cache should be surfaced, add expiresAt: { gte: new Date() }; otherwise document the chosen semantic.
  • 🟡 packages/features/apps/components/CredentialActionsDropdown.tsx (L43)
    deleteCacheMutation never invalidates connectedCalendars; it relies on an optional onChanged/onSuccess callback that is only wired in the CalendarListContainer context. In the EventAdvancedTab usage, after a successful cache deletion the dropdown keeps showing the stale "Cache Status / Last updated" block (query uses refetchOnWindowFocus: false) until a full reload. Mirror disconnectMutation and invalidate connectedCalendars (and arguably apps.integrations) in an onSettled.
  • 🟡 packages/app-store/googlecalendar/lib/CalendarService.ts (L1024)
    Dead-write concern: this call bumps SelectedCalendar.updatedAt for every calendar under the credential, but the new cache-status feature surfaces cacheUpdatedAt from CalendarCache.updatedAt via getCacheStatusByCredentialIds — not SelectedCalendar.updatedAt. The PR's own notes ask whether this update should be removed. It also runs on every webhook push/cache refresh, regardless of which calendar changed and even when selectedCalendars is empty. Remove it if unconsumed, or document it as intentional for future use.
  • 🟡 packages/app-store/googlecalendar/lib/CalendarService.ts (L1024)
    Delegated-credential path may no-op: in the webhook flow the service can be built from a delegation credential whose id differs from the credentialId on the SelectedCalendar rows, so updateManyByCredentialId(this.credential.id, {}) matches zero rows and updatedAt won't be bumped even though the cache was refreshed. Verify this.credential.id equals SelectedCalendar.credentialId in both normal and delegation paths.
  • 🟡 scripts/test-gcal-webhooks.sh (L68)
    sed -i '' -E ... is BSD/macOS-only; on GNU sed (most Linux/CI) -i '' is interpreted as -i with an empty inline-script filename plus '' as the script, so the GOOGLE_WEBHOOK_URL update fails. Use a portable form (sed -i.bak + rm, or temp file + mv), and quote the variable.
  • 🟡 scripts/test-gcal-webhooks.sh (L17)
    cleanup() ends with exit 0 and is invoked on every error path (rate-limit branch, tunnel-URL extraction failure), so a script that fails to obtain a tunnel URL still returns success, silently breaking CI/automation keyed on exit status. Return a non-zero code on failure paths.
  • 🟡 scripts/test-gcal-webhooks.sh (L28)
    The reuse branch relies on pgrep -f "tmole $TM_PORT" (may match stale/unrelated processes and return multiple PIDs) and re-reads a possibly missing/stale $LOG_FILE, silently falling into the "Failed to extract Tunnelmole URL" path and exiting 0 without cleaning up. Validate the process is alive and the URL current, or fail loudly.
Small things (take or leave)
  • 🔵 packages/trpc/server/routers/viewer/calendars/_router.tsx (L28)
    The deleteCache schema is defined inline (z.object({ credentialId: z.number() })) whereas every other procedure in this router uses a dedicated *.schema.ts file; also z.number() should be z.number().int().positive() so fractional/negative ids are rejected early instead of silently failing the findFirst lookup.
  • 🔵 packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts (L35)
    cacheStatusMap.get(calendar.credentialId) || null should use nullish coalescing ?? null; || works only because a Date is always truthy and is fragile if the type changes.
  • 🔵 packages/features/calendar-cache/calendar-cache.repository.ts (L173)
    getCacheStatusByCredentialIds(credentialIds) performs no ownership (userId) scoping. It is safe today only because the sole caller passes user-scoped ids. As a public repository API it should document the "caller must pre-scope by userId" contract to avoid future metadata leakage.
  • 🔵 packages/features/calendar-cache/calendar-cache.repository.ts (L1)
    A pre-existing TODO in getCachedAvailability states updatedAt "doesn't exist yet in CalendarCache table"; this MR adds exactly that column, so the TODO is now stale and should be revisited/resolved.
  • 🔵 packages/prisma/migrations/20250715160635_add_calendar_cache_updated_at/migration.sql (L4)
    The auto-generated Prisma warning header still says the column was added "without a default value ... not possible if the table is not empty," but the actual statement adds it NOT NULL DEFAULT NOW(). The warning text is now contradictory/misleading and should be updated or removed.
  • 🔵 packages/features/apps/components/CredentialActionsDropdown.tsx (L150)
    Copy-paste bug: the disconnect dialog's ConfirmationDialogContent.onConfirm calls setDeleteModalOpen(false) instead of setDisconnectModalOpen(false). It still closes via the DialogClose but leaves the disconnect modal state unmanaged and will misbehave if the closing mechanism changes.
  • 🔵 packages/features/apps/components/CredentialActionsDropdown.tsx (L89)
    The timestamp is hardcoded to new Intl.DateTimeFormat("en-US", ...), disregarding the user's active locale (the component already calls useLocale()). Format with the user's locale; the interpolation: { escapeValue: false } is also unnecessary for a pre-formatted string.
  • 🔵 packages/features/apps/components/CredentialActionsDropdown.tsx (L135)
    Both confirmation dialogs close synchronously in onConfirm and don't wire isPending/loadingText, so if the mutation fails the user gets only a toast, and rapid double-triggering is possible. Wire isPending={deleteCacheMutation.isPending} / isPending={disconnectMutation.isPending} and close on success per the established pattern.
  • 🔵 packages/features/apps/components/CredentialActionsDropdown.tsx (L84)
    The read-only cache-status header ("Cache Status" + "Last updated") is rendered inside a focusable/interactive DropdownMenuItem, giving it an erroneous affordance (focus ring, hover highlight, closes menu on activation). Render it as a non-interactive block (e.g. DropdownMenuLabel or plain div) separate from the destructive actions.
  • 🔵 scripts/test-gcal-webhooks.sh (L1)
    No set -euo pipefail; unset variables (OWNED_PID, TMOLE_PID) are silently treated as empty and pipeline errors don't fail. Add set -euo pipefail and initialize the PID vars explicitly.
  • 🔵 scripts/test-gcal-webhooks.sh (L46)
    The wait loop only greps for the exact "limited to 10 tunnels per hour" string; any other failure (port in use, tmole not installed, network/auth error) just times out and reports "Failed to extract URL". Check the spawned process is alive (kill -0) and handle non-matching logs with a clear early error.
  • 🔵 scripts/test-gcal-webhooks.sh (L5)
    ENV_FILE="../.env" is relative to the CWD and tmole $TM_PORT is unquoted. Resolve ENV_FILE relative to the script ($(dirname "$0")/../.env) and quote tmole "$TM_PORT".
  • 🔵 apps/web/package.json (L11)
    The dev:cron script was changed to npx tsx cron-tester.ts, but tsx is not declared anywhere in the monorepo; npx tsx fetches an unpinned version at first run, bypassing the lockfile, and diverges from every other TS script which uses the declared ts-node. Either revert to ts-node or add tsx to apps/web devDependencies (and consider migrating the sibling ts-node scripts). This edit is also unrelated to the MR's stated feature.

✅ Feature-level checklist

Looks good

  • ✅ Verify the deleteCache tRPC mutation exists and is wired to the UI component.
    The trpc.viewer.calendars.deleteCache mutation exists in the router and is referenced by CredentialActionsDropdown. However, its error handling (plain Error instead of TRPCError) and schema hardening should be improved — these are quality issues, not missing functionality.

Not there yet

  • ❌ End-to-end testing with Google Calendar: install integration, verify dropdown, cache status, and deletion with confirmation works.
    The PR notes state local testing was limited due to no installed calendar integrations. More importantly, a confirmed runtime bug — updateManyByCredentialId(this.credential.id, {}) with empty data throwing a Prisma validation error in the Google webhook/availability flow (CalendarService.ts:1024) — would break the E2E flow, so this acceptance criterion cannot be considered satisfied.

Partially covered

  • ⚠️ Test cache status display logic: cacheUpdatedAt timestamps correctly formatted and displayed only for Google Calendar credentials with actual cache data.
    The conditional rendering (Google-only, with cache) is correct, but the timestamp is hardcoded to the en-US locale instead of the user's locale, and getCacheStatusByCredentialIds counts expired cache rows, so an expired cache still shows "Last updated" as if usable.
  • ⚠️ Confirm repository pattern correctness: getCacheStatusByCredentialIds() returns accurate data and the credential-to-cache mapping works in the UI.
    The mapping is O(n) and credentialId->max updatedAt is correct overall, but accuracy is affected by the expired-rows count, the repository method has no documented userId scoping contract, and both deleteCache.handler.ts and connectedCalendars.handler.ts bypass the CalendarCache.init() repository/feature-flag abstraction — diverging from the established pattern (the mock's deleteManyByCredential is implemented but never used).
  • ⚠️ Verify dropdown conditional rendering: only appears for Google Calendar credentials and handles edge cases (no cache, disabled integrations, etc.).
    The Google-only/no-cache gating is correct and the wrapper routes delegationCredentialId/disableConnectionModification through both branches. However, a delegation edge case remains: the dropdown can render "Delete cached data" for a delegated Google credential whose credentialId belongs to the delegating user, while the handler denies the deletion with an ownership error — the intended UX must be confirmed.
  • ⚠️ PR note: consider whether the SelectedCalendar.updatedAt update should be removed, since the frontend uses cacheUpdatedAt from CalendarCache.
    The review confirms the concern: CalendarService.ts:1024 bumps SelectedCalendar.updatedAt on every webhook/cache refresh, but the feature consumes CalendarCache.updatedAt. This appears to be a redundant per-event DB write and should either be removed or documented as intentional for future use; additionally, the delegated-credential path may silently no-op this update.