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

[CodingKylo]

Martian Code Review Benchmark PR (mirrored from source ai-code-review-evaluation#11)

[re-entry-ai]

🛡️ re-entry.ai Code Review

@CodingKylo — Please review the following assessment:

🚨 Risk Score: 92/100 · CRITICAL

Dimension	Level
Likelihood	🔴 Critical
Impact	🔴 Critical
Detectability	🟠 High

Intent

Add calendar cache status (based on CalendarCache.updatedAt) to connected calendars and expose UI actions to delete cached calendar data.

Summary

The PR enriches the connected calendars TRPC response with per-credential cache status (via a new CalendarCacheRepository.getCacheStatusByCredentialIds groupBy query) and adds a new client dropdown that can call a new authed TRPC mutation to delete calendar cache rows. The highest risk is correctness/contract drift: the cache-status enrichment relies on updatedAt semantics and on how missing groupBy results are mapped to null, and the “touch” path uses updateMany with an empty update object which may not reliably update updatedAt. The reviewer must verify server-side authorization for deleteCache is sufficient, and verify that cacheUpdatedAt is computed consistently (missing vs null) and actually changes when cache is refreshed.

🎯 Review Focus

Confirm server-side authorization and data scoping for deleteCache (the deleteMany must be provably limited to the authenticated user’s credential/cache rows), and confirm that CalendarCache.updatedAt is reliably updated by the cache refresh/touch path so the UI’s cacheUpdatedAt-driven actions reflect reality.

Key Findings

• ⚠️ [packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts:L24] WARNING: cache-status enrichment maps missing groupBy results to null using || null: cacheUpdatedAt: cacheStatusMap.get(calendar.credentialId) || null, — If updatedAt can ever be new Date(0) or another falsy-like value (rare but possible with custom Date handling), || would incorrectly coerce it to null. Fix: use nullish coalescing: cacheUpdatedAt: cacheStatusMap.get(calendar.credentialId) ?? null.

✅ Action Checklist

• [ ] WARNING [packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts:L24] — cache-status enrichment maps missing groupBy results to null using || null: cacheUpdatedAt: cacheStatusMap.get(calendar.credentialId) || null, — If updatedAt can ever be new Date(0) or another falsy-like value (rare but possible with custom Date handling), || would incorrectly coerce it to null. Fix: use nullish coalescing: cacheUpdatedAt: cacheStatusMap.get(calendar.credentialId) ?? null.
• [ ] SUGGESTION — Verify the Prisma “touch” semantics for updatedAt: packages/app-store/googlecalendar/lib/CalendarService.ts calls SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {}) after setAvailabilityInCache. If Prisma rejects empty updates or doesn’t trigger @updatedAt, cacheUpdatedAt will never change and the UI actions will be wrong. Fix: ensure the repository update sets a concrete field (or uses a supported no-op that still updates updatedAt). Example: await prisma.selectedCalendar.updateMany({ where: { credentialId }, data: { updatedAt: new Date() } }) (or whatever model/field actually drives the cache status).
• [ ] SUGGESTION — Make the cache-status contract explicit and consistent: getCacheStatusByCredentialIds returns only credentials that have at least one CalendarCache row (groupBy omits missing groups). Downstream code then treats “missing” as null. Fix: either (a) return a complete array aligned to input ids (fill missing with updatedAt: null), or (b) rename the field to reflect semantics (e.g., hasCache boolean) and compute it server-side to avoid consumer ambiguity. Implement in packages/features/calendar-cache/calendar-cache.repository.ts and adjust connectedCalendars.handler.ts accordingly.
• [ ] SUGGESTION — Avoid dynamic import/type drift across the router/handler boundary: deleteCache is wired in _router.tsx to deleteCacheHandler({ ctx, input }), but the handler’s DeleteCacheOptions is local. Fix: export the Zod input schema (or the inferred input type) from deleteCache.handler.ts and import it in _router.tsx, or directly reference the handler function so TS can validate the call signature at compile time.

Suggestions

• Verify the Prisma “touch” semantics for updatedAt: packages/app-store/googlecalendar/lib/CalendarService.ts calls SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {}) after setAvailabilityInCache. If Prisma rejects empty updates or doesn’t trigger @updatedAt, cacheUpdatedAt will never change and the UI actions will be wrong. Fix: ensure the repository update sets a concrete field (or uses a supported no-op that still updates updatedAt). Example: await prisma.selectedCalendar.updateMany({ where: { credentialId }, data: { updatedAt: new Date() } }) (or whatever model/field actually drives the cache status).
• Make the cache-status contract explicit and consistent: getCacheStatusByCredentialIds returns only credentials that have at least one CalendarCache row (groupBy omits missing groups). Downstream code then treats “missing” as null. Fix: either (a) return a complete array aligned to input ids (fill missing with updatedAt: null), or (b) rename the field to reflect semantics (e.g., hasCache boolean) and compute it server-side to avoid consumer ambiguity. Implement in packages/features/calendar-cache/calendar-cache.repository.ts and adjust connectedCalendars.handler.ts accordingly.
• Avoid dynamic import/type drift across the router/handler boundary: deleteCache is wired in _router.tsx to deleteCacheHandler({ ctx, input }), but the handler’s DeleteCacheOptions is local. Fix: export the Zod input schema (or the inferred input type) from deleteCache.handler.ts and import it in _router.tsx, or directly reference the handler function so TS can validate the call signature at compile time.

📝 This review includes 4 inline comments (1 warning, 3 notes)

---

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

[re-entry-ai]

⚠️ WARNING

This is a client component that invokes trpc.viewer.*.useMutation() directly, but the real security boundary is the server procedure. The UI gating (canDisconnect / hasCache) only hides controls; it does not prevent a caller from invoking the mutation. This is only a critical issue if the underlying deleteCache / credentials.delete procedures lack server-side authorization, so the finding should be framed as a server-auth dependency rather than a UI bypass bug.

---

re-entry.ai

[re-entry-ai]

💡 NOTE

The handler authorizes the credential by id and userId, then deletes calendarCache rows by the same credentialId. That is a reasonable ownership check, so the concern about an identifier mismatch is speculative from this diff alone. The real risk here is not visible in the patch unless the schema allows cache rows to be created without a matching credential ownership relationship.

---

re-entry.ai

[re-entry-ai]

💡 NOTE

This adds a cache-status lookup for every connected calendar, but the code does not guard the empty-array case before calling getCacheStatusByCredentialIds. In practice this is likely harmless with Prisma in: [], but it is still an unnecessary query path and makes the handler depend on repository behavior for an edge case that can be handled locally.

---

re-entry.ai

[re-entry-ai]

💡 NOTE

hasCache is typed as Date | false because cacheUpdatedAt is a Date | null | undefined. That works in JSX truthiness checks, but it leaks a non-boolean into later logic and obscures intent. Coercing it to a boolean would make the control flow clearer and avoid accidental misuse.

---

re-entry.ai

[re-entry-ai]

🛡️ re-entry.ai Code Review

@CodingKylo — Please review the following assessment:

🚨 Risk Score: 91/100 · CRITICAL

Dimension	Level
Likelihood	🔴 Critical
Impact	🔴 Critical
Detectability	🟠 High

Summary

Code Review Summary

Risk Level: CRITICAL (Score: 91/100)
Files Changed: 0
High Risk Areas: 3

Key Concerns

• CHANGEMAGNITUDE (critical): The PR touches many areas (UI, TRPC, repository, Prisma schema/migration, and calendar service behavior), increasing the chance of integration bugs.
• DATA (high): A Prisma schema change adds a new non-null column with a default via migration, which can still affect existing data and downstream queries.
• CONFIGURATION (high): The new cache status UI depends on updatedAt being correctly maintained, and the CalendarService updateMany call uses an empty update object which may not actually update timestamps as intended.

📝 This review includes 3 inline comments (1 warning, 2 notes)

---

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

[re-entry-ai]

⚠️ WARNING

The UI now exposes destructive calendar actions from a client component, but the real security boundary is the TRPC procedures it calls. This is only safe if viewer.calendars.deleteCache and viewer.credentials.delete enforce tenant/user authorization server-side; otherwise the dropdown can be bypassed entirely by invoking the mutations directly. The root issue is relying on UI state for access control instead of treating the server procedures as the enforcement point.

---

re-entry.ai

[re-entry-ai]

💡 NOTE

This handler does perform a basic ownership check on the credential before deleting cache rows, so the original concern about missing authorization is not substantiated by the diff. However, the check is only against credential.userId, which means the security boundary depends on credentialId being sufficient to identify the tenant-owned credential in all cases. If delegated/shared credentials are introduced elsewhere, this should be validated against the same ownership model used by the rest of the credentials router.

---

re-entry.ai

[re-entry-ai]

💡 NOTE

new CalendarCacheRepository() is a real wiring concern only if the repository has hidden dependencies, but the diff shows this repository already uses the shared Prisma singleton internally and the new method is a simple read helper. So the constructor-instantiation warning is not supported by the change itself. The more relevant architectural issue is that this handler now mixes calendar retrieval with cache-status aggregation via a second repository call, which increases coupling but is not a correctness bug.

---

re-entry.ai

[re-entry-ai]

🛡️ re-entry.ai Code Review

@CodingKylo — Please review the following assessment:

🚨 Risk Score: 91/100 · CRITICAL

Dimension	Level
Likelihood	🔴 Critical
Impact	🔴 Critical
Detectability	🟠 High

Intent

Add UI and backend support for showing calendar cache status per connected credential and allowing an authenticated user to delete cached calendar data.

Summary

The PR introduces a new Prisma column for cache updatedAt, a repository method to compute cache status by credentialId, and a new authed TRPC mutation to delete calendarCache rows. The connected-calendars handler now performs an additional grouped query and enriches results with cacheUpdatedAt, which the new dropdown UI uses to display status and trigger deletion. The highest risks to verify are (1) a likely runtime crash due to assuming connectedCalendar.integration is always present in the wrapper, and (2) correctness of the “touch” behavior for updatedAt—the calendar service updateMany call uses an empty update object, which may be a no-op depending on Prisma behavior. Also verify the TRPC handler wiring/typing matches the actual authedProcedure ctx shape to avoid runtime failures.

🎯 Review Focus

Confirm that connectedCalendar.integration is always present (or make the wrapper null-safe) and that the code paths that are supposed to “touch” cache timestamps actually update CalendarCache.updatedAt/the relevant timestamp—otherwise the UI status will be wrong or the page will crash.

Key Findings

• 🚨 [packages/app-store/googlecalendar/lib/CalendarService.ts:L1019-1022] CRITICAL: updateManyByCredentialId(this.credential.id, {}) — Passing an empty update object is very likely a no-op or invalid for Prisma updateMany/update inputs, meaning SelectedCalendar.updatedAt (or the intended cache “touch” timestamp) may never change. That breaks the core feature: cache status based on updatedAt will be stale and misleading. Fix: implement a dedicated “touch” update that sets a real field or explicitly updates the timestamp. For example, in the repository: await prisma.selectedCalendar.updateMany({ where: { credentialId }, data: { updatedAt: new Date() } }) (or set a harmless field) and ensure the repository method signature requires non-empty data.
• ⚠️ [packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts:L23-L35] WARNING: const cacheStatuses = await cacheRepository.getCacheStatusByCredentialIds(credentialIds); + cacheStatuses.map((cache) => [cache.credentialId, cache.updatedAt]) — This adds an extra DB round-trip and in-memory enrichment for every connected-calendars request. Under load, this can become a measurable latency regression. Fix: fold the cache status into the existing query (e.g., join/include if your schema allows) or at least ensure credentialIds is deduped and the query is bounded. Example: const credentialIds = [...new Set(connectedCalendars.map(c => c.credentialId))]; and consider caching the status per request/user.

✅ Action Checklist

• [ ] CRITICAL [packages/app-store/googlecalendar/lib/CalendarService.ts:L1019-1022] — updateManyByCredentialId(this.credential.id, {}) — Passing an empty update object is very likely a no-op or invalid for Prisma updateMany/update inputs, meaning SelectedCalendar.updatedAt (or the intended cache “touch” timestamp) may never change. That breaks the core feature: cache status based on updatedAt will be stale and misleading. Fix: implement a dedicated “touch” update that sets a real field or explicitly updates the timestamp. For example, in the repository: await prisma.selectedCalendar.updateMany({ where: { credentialId }, data: { updatedAt: new Date() } }) (or set a harmless field) and ensure the repository method signature requires non-empty data.
• [ ] WARNING [packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts:L23-L35] — const cacheStatuses = await cacheRepository.getCacheStatusByCredentialIds(credentialIds); + cacheStatuses.map((cache) => [cache.credentialId, cache.updatedAt]) — This adds an extra DB round-trip and in-memory enrichment for every connected-calendars request. Under load, this can become a measurable latency regression. Fix: fold the cache status into the existing query (e.g., join/include if your schema allows) or at least ensure credentialIds is deduped and the query is bounded. Example: const credentialIds = [...new Set(connectedCalendars.map(c => c.credentialId))]; and consider caching the status per request/user.
• [ ] SUGGESTION — [packages/trpc/server/routers/viewer/calendars/_router.tsx:L24-L33] WARNING: Verify handler ctx/input typing matches authedProcedure. The handler defines type DeleteCacheOptions = { ctx: { user: NonNullable<TrpcSessionUser> }; input: { credentialId: number } }, but _router.tsx passes the raw ctx from authedProcedure. If the ctx shape differs, this can compile but fail at runtime. Fix: import and reuse the same ctx type used by other handlers in this router layer (e.g., AuthedCtx / TrpcSessionUser ctx type) so the handler signature matches exactly.
• [ ] SUGGESTION — [packages/features/calendar-cache/calendar-cache.repository.mock.ts:L1] WARNING: The repository mock returns [] unconditionally (per cross-file analysis). This can cause tests to silently pass while production shows cache status and enables/disables actions differently. Fix: update the mock to return deterministic statuses based on input credentialIds (even if it returns updatedAt: null), or adjust tests to explicitly cover the empty-state UI behavior.
• [ ] SUGGESTION — [packages/features/calendar-cache/calendar-cache.repository.ts:L169-L196] INFO: Consider handling empty credentialIds to avoid an unnecessary groupBy call. Fix: if (credentialIds.length === 0) return []; before calling Prisma.

Suggestions

• [packages/trpc/server/routers/viewer/calendars/_router.tsx:L24-L33] WARNING: Verify handler ctx/input typing matches authedProcedure. The handler defines type DeleteCacheOptions = { ctx: { user: NonNullable<TrpcSessionUser> }; input: { credentialId: number } }, but _router.tsx passes the raw ctx from authedProcedure. If the ctx shape differs, this can compile but fail at runtime. Fix: import and reuse the same ctx type used by other handlers in this router layer (e.g., AuthedCtx / TrpcSessionUser ctx type) so the handler signature matches exactly.
• [packages/features/calendar-cache/calendar-cache.repository.mock.ts:L1] WARNING: The repository mock returns [] unconditionally (per cross-file analysis). This can cause tests to silently pass while production shows cache status and enables/disables actions differently. Fix: update the mock to return deterministic statuses based on input credentialIds (even if it returns updatedAt: null), or adjust tests to explicitly cover the empty-state UI behavior.
• [packages/features/calendar-cache/calendar-cache.repository.ts:L169-L196] INFO: Consider handling empty credentialIds to avoid an unnecessary groupBy call. Fix: if (credentialIds.length === 0) return []; before calling Prisma.

📝 This review includes 4 inline comments (1 critical, 3 notes)

---

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

[re-entry-ai]

💡 NOTE

The client boundary itself is fine; the real risk here is authorization, not `

---

re-entry.ai

[re-entry-ai]

🚨 CRITICAL

This is a real nullability regression: the new prop access connectedCalendar.integration.type assumes integration is always present, but the previous code path only used integration.description in a context where optional chaining was already needed for other fields. If any connected calendar can lack an integration object, this will throw during render. The root cause is that the wrapper now forwards a required integrationType prop without preserving the previous null-safe access pattern.

---

re-entry.ai

[re-entry-ai]

💡 NOTE

The new repository import is used, so this is not an unused-import issue. However, the handler now performs an extra grouped query for every connected-calendars request and enriches each row in memory. That is a real performance/correctness tradeoff, but not a build failure.

---

re-entry.ai

[re-entry-ai]

💡 NOTE

The authorization check is scoped to the credential owner before the destructive delete, so the concern about deleting by credentialId alone is mitigated. The remaining issue is that the delete is not additionally constrained by userId, but the preceding ownership lookup makes this a defense-in-depth improvement rather than a security bug in the current diff.

---

re-entry.ai

[re-entry-ai]

🛡️ re-entry.ai Code Review

@CodingKylo — Please review the following assessment:

🚨 Risk Score: 91/100 · CRITICAL

Dimension	Level
Likelihood	🔴 Critical
Impact	🔴 Critical
Detectability	🟠 High

Intent

Add UI and backend support for showing calendar cache freshness per connected credential and allow users to delete cached calendar data.

Summary

The PR enriches the connected-calendars response with a per-credential cacheUpdatedAt derived from a new Prisma column, and introduces a new viewer TRPC mutation to delete calendar cache rows. The top risks are (1) a correctness/authorization drift in deleteCacheHandler: it verifies credential ownership but deletes by credentialId without re-scoping the delete to the verified credential, and (2) cache-status enrichment that does not guarantee an entry for every requested credentialId, which can desync UI state. You should verify the delete mutation’s authorization semantics, the repository/handler contract for cache-status completeness (including empty credentialIds), and that the Prisma migration/updatedAt behavior matches the intended “freshness” semantics.

🎯 Review Focus

The authorization semantics and data-contract correctness around cache deletion and cache-status enrichment: verify deleteCacheHandler’s delete scoping cannot drift from the ownership check, and verify cache-status returns a deterministic entry per requested credentialId (including empty credentialIds) so the UI state is accurate.

✅ Action Checklist

• [ ] SUGGESTION — [packages/trpc/server/routers/viewer/calendars/deleteCache.handler.ts:L18] WARNING: throwing a plain Error (throw new Error("Credential not found or access denied")) will typically surface as an internal server error instead of a typed TRPC error (hurts client handling and observability). Fix: throw a TRPC error with correct code.

Example:

import { TRPCError } from "@trpc/server";
...
if (!credential) {
  throw new TRPCError({ code: "NOT_FOUND" });
}

• [ ] SUGGESTION — [packages/features/calendar-cache/calendar-cache.repository.ts:L169] WARNING: getCacheStatusByCredentialIds uses groupBy and returns only credentials that have at least one calendarCache row; it never returns updatedAt: null for missing credentials. This forces the handler to guess via Map.get(...) || null, which can conflate “no cache rows” with “cache rows exist but updatedAt is null” (and makes it impossible to distinguish “unknown” vs “never cached”). Fix: either (a) change the repository to return a complete list keyed by input credentialIds, or (b) change the handler to merge against the input list explicitly and treat missing as a deliberate state.

Concrete fix (repository):

const byId = new Map(cacheStatuses.map(s => [s.credentialId, s.updatedAt]));
return credentialIds.map(id => ({ credentialId: id, updatedAt: byId.get(id) ?? null }));

• [ ] SUGGESTION — [packages/app-store/googlecalendar/lib/CalendarService.ts:L1019] CRITICAL (verify in diff): updateManyByCredentialId(this.credential.id, {}) relies on Prisma accepting an empty update object; if Prisma rejects empty updates, webhook-driven cache freshness will break. Fix: ensure the update method sets at least one field (e.g., updatedAt: new Date()), or provide a dedicated method that only updates updatedAt.

Example:

await selectedCalendarRepository.updateManyByCredentialId(this.credential.id, { updatedAt: new Date() });

Suggestions

• [packages/trpc/server/routers/viewer/calendars/deleteCache.handler.ts:L18] WARNING: throwing a plain Error (throw new Error("Credential not found or access denied")) will typically surface as an internal server error instead of a typed TRPC error (hurts client handling and observability). Fix: throw a TRPC error with correct code.

Example:

import { TRPCError } from "@trpc/server";
...
if (!credential) {
  throw new TRPCError({ code: "NOT_FOUND" });
}

• [packages/features/calendar-cache/calendar-cache.repository.ts:L169] WARNING: getCacheStatusByCredentialIds uses groupBy and returns only credentials that have at least one calendarCache row; it never returns updatedAt: null for missing credentials. This forces the handler to guess via Map.get(...) || null, which can conflate “no cache rows” with “cache rows exist but updatedAt is null” (and makes it impossible to distinguish “unknown” vs “never cached”). Fix: either (a) change the repository to return a complete list keyed by input credentialIds, or (b) change the handler to merge against the input list explicitly and treat missing as a deliberate state.

Concrete fix (repository):

const byId = new Map(cacheStatuses.map(s => [s.credentialId, s.updatedAt]));
return credentialIds.map(id => ({ credentialId: id, updatedAt: byId.get(id) ?? null }));

• [packages/app-store/googlecalendar/lib/CalendarService.ts:L1019] CRITICAL (verify in diff): updateManyByCredentialId(this.credential.id, {}) relies on Prisma accepting an empty update object; if Prisma rejects empty updates, webhook-driven cache freshness will break. Fix: ensure the update method sets at least one field (e.g., updatedAt: new Date()), or provide a dedicated method that only updates updatedAt.

Example:

await selectedCalendarRepository.updateManyByCredentialId(this.credential.id, { updatedAt: new Date() });

📝 This review includes 2 inline comments (1 warning, 1 note)

---

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

[re-entry-ai]

⚠️ WARNING

This is a real correctness issue: the handler verifies ownership on credential, but the actual delete is only scoped by credentialId. Because the route is viewer-scoped, the safer pattern is to delete through the already-authorized credential record (or include userId in the delete filter) so the authorization check and the destructive operation cannot drift apart. Also, throwing a plain Error here will typically surface as an internal server error instead of a typed TRPC authorization/not-found error, which makes client handling and observability worse.

---

re-entry.ai

[re-entry-ai]

💡 NOTE

The new cache-status lookup is executed unconditionally, even when connectedCalendars is empty. In that case the repository call receives an empty credentialIds array and still performs a grouped query. This is not a security bug, but it is an avoidable database round-trip and should be short-circuited for the empty case. The root cause is that the enrichment logic was added without guarding the zero-result path.

---

re-entry.ai

[re-entry-ai]

🛡️ re-entry.ai Code Review

@CodingKylo — Please review the following assessment:

🚨 Risk Score: 92/100 · CRITICAL

Dimension	Level
Likelihood	🔴 Critical
Impact	🔴 Critical
Detectability	🟠 High

Intent

Add calendar cache status enrichment to connected calendars and provide a UI/TRPC action to delete the calendar cache (alongside existing disconnect/remove actions).

Summary

The PR introduces a new cache status lookup (via Prisma groupBy) to enrich connected calendars with cacheUpdatedAt, and adds a new TRPC mutation/handler to delete calendarCache rows for a credential. It also replaces the previous call-site gating for disconnectability with a shared CredentialActionsDropdown component. The top risks are (1) a UI authorization/visibility regression where delegation credentials may become disconnectable if the dropdown doesn’t perfectly replicate prior gating, and (2) correctness/robustness issues around empty credentialId lists and repository/mocking/contract mismatches. Verify the dropdown preserves the delegation-disconnect rule, the cache status enrichment handles empty inputs safely, and the repository interface/mock signatures compile and behave consistently.

🎯 Review Focus

The most dangerous area is the UI authorization/visibility regression introduced by removing call-site delegation gating and delegating disconnectability rules to CredentialActionsDropdown; confirm the dropdown cannot expose disconnect/remove for delegation credentials and add tests to lock this down.

Key Findings

• ⚠️ [packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts:L24] WARNING: Cache enrichment builds credentialIds from connectedCalendars and calls getCacheStatusByCredentialIds(credentialIds) without guarding against an empty array; depending on Prisma/DB behavior, in: [] can be error-prone or yield unexpected results. Fix by short-circuiting: if (credentialIds.length === 0) return { connectedCalendars: [], destinationCalendar }; (or set cacheUpdatedAt to null for all calendars without querying).
• ⚠️ [packages/features/calendar-cache/calendar-cache.repository.ts:L169] WARNING: getCacheStatusByCredentialIds uses groupBy with where: { credentialId: { in: credentialIds } } and then maps _max.updatedAt; if credentialIds is empty, this can produce inconsistent behavior across Prisma versions/DBs and adds unnecessary DB work. Fix by early-returning when input is empty: if (credentialIds.length === 0) return []; and consider de-duping ids before querying.

✅ Action Checklist

• [ ] WARNING [packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts:L24] — Cache enrichment builds credentialIds from connectedCalendars and calls getCacheStatusByCredentialIds(credentialIds) without guarding against an empty array; depending on Prisma/DB behavior, in: [] can be error-prone or yield unexpected results. Fix by short-circuiting: if (credentialIds.length === 0) return { connectedCalendars: [], destinationCalendar }; (or set cacheUpdatedAt to null for all calendars without querying).
• [ ] WARNING [packages/features/calendar-cache/calendar-cache.repository.ts:L169] — getCacheStatusByCredentialIds uses groupBy with where: { credentialId: { in: credentialIds } } and then maps _max.updatedAt; if credentialIds is empty, this can produce inconsistent behavior across Prisma versions/DBs and adds unnecessary DB work. Fix by early-returning when input is empty: if (credentialIds.length === 0) return []; and consider de-duping ids before querying.
• [ ] SUGGESTION — Fix the delegation disconnect gating regression by either restoring the wrapper-level conditional or enforcing it inside CredentialActionsDropdown and adding a unit/integration test. File: packages/platform/atoms/selected-calendars/wrappers/SelectedCalendarsSettingsWebWrapper.tsx:L67-L83 (restore the !connectedCalendar.delegationCredentialId check).
• [ ] SUGGESTION — Harden the cache enrichment path for empty inputs and ensure consistent output shape. File: packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts:L24-L38: add if (credentialIds.length === 0) { return { connectedCalendars: connectedCalendars.map(c => ({...c, cacheUpdatedAt: null})), destinationCalendar }; }.
• [ ] SUGGESTION — Verify repository contract + mocks compile: the systemic analysis indicates getCacheStatusByCredentialIds signature mismatch in calendar-cache.repository.mock.ts. Update CalendarCacheRepositoryMock.getCacheStatusByCredentialIds(credentialIds: number[]) to accept the parameter and return the same { credentialId, updatedAt }[] shape.
• [ ] SUGGESTION — Standardize TRPC error handling for deleteCache so the UI can reliably interpret failures. File: packages/trpc/server/routers/viewer/calendars/deleteCache.handler.ts:L11-L27: replace throw new Error("Credential not found or access denied") with a TRPCError (e.g., new TRPCError({ code: 'NOT_FOUND' })) and ensure the UI toast logic maps to those codes.

Suggestions

• Fix the delegation disconnect gating regression by either restoring the wrapper-level conditional or enforcing it inside CredentialActionsDropdown and adding a unit/integration test. File: packages/platform/atoms/selected-calendars/wrappers/SelectedCalendarsSettingsWebWrapper.tsx:L67-L83 (restore the !connectedCalendar.delegationCredentialId check).
• Harden the cache enrichment path for empty inputs and ensure consistent output shape. File: packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts:L24-L38: add if (credentialIds.length === 0) { return { connectedCalendars: connectedCalendars.map(c => ({...c, cacheUpdatedAt: null})), destinationCalendar }; }.
• Verify repository contract + mocks compile: the systemic analysis indicates getCacheStatusByCredentialIds signature mismatch in calendar-cache.repository.mock.ts. Update CalendarCacheRepositoryMock.getCacheStatusByCredentialIds(credentialIds: number[]) to accept the parameter and return the same { credentialId, updatedAt }[] shape.
• Standardize TRPC error handling for deleteCache so the UI can reliably interpret failures. File: packages/trpc/server/routers/viewer/calendars/deleteCache.handler.ts:L11-L27: replace throw new Error("Credential not found or access denied") with a TRPCError (e.g., new TRPCError({ code: 'NOT_FOUND' })) and ensure the UI toast logic maps to those codes.

📝 This review includes 3 inline comments (1 critical, 2 warnings)

---

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

[re-entry-ai]

🚨 CRITICAL

Quote: <CredentialActionsDropdown credentialId={connectedCalendar.credentialId} integrationType={connectedCalendar.integration.type} cacheUpdatedAt={connectedCalendar.cacheUpdatedAt} onSuccess={onChanged} delegationCredentialId={connectedCalendar.delegationCredentialId} disableConnectionModification={disableConnectionModification} />

Issue: The previous UI explicitly prevented showing DisconnectIntegration when connectedCalendar.delegationCredentialId is set ("Delegation credential can't be disconnected"). This new code always renders CredentialActionsDropdown regardless of delegationCredentialId, relying on the dropdown to enforce the rule. If the dropdown does not fully replicate the prior gating, delegation credentials could become disconnectable.

Fix: Preserve the original gating at the call site, e.g.:

{!connectedCalendar.delegationCredentialId && (
  <div className="flex w-32 justify-end">
    <CredentialActionsDropdown ... />
  </div>
)}

Or ensure CredentialActionsDropdown internally blocks the disconnect action when delegationCredentialId is truthy (and add/verify tests). (see also L74)

---

re-entry.ai

[re-entry-ai]

⚠️ WARNING

Quote: import { CalendarCacheRepository } from "@calcom/features/calendar-cache/calendar-cache.repository";

Issue: This new import is introduced to support cache status enrichment. If CalendarCacheRepository is not actually used in the file (or only used in a type-only position), TypeScript/ESLint can fail the build due to an unused import.

Fix: Ensure the import is used as a value (it appears to be used via new CalendarCacheRepository() in the diff). If not, remove the import or change to import type accordingly.

---

re-entry.ai

[re-entry-ai]

⚠️ WARNING

Quote: import { prisma } from "@calcom/prisma";

Issue: This file introduces a new Prisma dependency. If prisma is not used elsewhere in the file beyond this handler, it’s fine; however, if the project expects a different Prisma client import pattern (e.g., getPrisma() or a namespaced client), this could break at runtime/build. The diff alone can’t confirm, but the new import is a potential integration risk.

Fix: Verify the correct Prisma client import for this codebase and ensure prisma is the intended instance (e.g., match other handlers in the same folder).

---

re-entry.ai