fix: dynamic adapter imports not working in self-hosted instances#23784
fix: dynamic adapter imports not working in self-hosted instances#23784anikdhabal merged 5 commits intocalcom:mainfrom
Conversation
|
@naaa760 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
Walkthrough
Possibly related PRs
Pre-merge checks (5 passed)✅ Passed checks (5 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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 |
dosubot
bot
added
app-store
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/app-store-cli/src/build.ts (1)
380-381: Avoid brittle string replace; centralize the E2E gating wrapper.The three ad-hoc
replacecalls are easy to drift. Factor a tiny helper to generate the gated export header once and reuse.Apply this minimal refactor:
@@ - // Find the export line and wrap it with E2E conditional + // Find the export line and wrap it with E2E conditional + const gateExportHeader = (name: string) => + `export const ${name} = process.env.NEXT_PUBLIC_IS_E2E === '1' ? {} : {`; @@ - exportLine.replace( - "export const CalendarServiceMap = {", - "export const CalendarServiceMap = process.env.NEXT_PUBLIC_IS_E2E === '1' ? {} : {" - ), + exportLine.replace(`export const CalendarServiceMap = {`, gateExportHeader("CalendarServiceMap")), @@ - exportLine.replace( - "export const AnalyticsServiceMap = {", - "export const AnalyticsServiceMap = process.env.NEXT_PUBLIC_IS_E2E === '1' ? {} : {" - ), + exportLine.replace(`export const AnalyticsServiceMap = {`, gateExportHeader("AnalyticsServiceMap")), @@ - exportLine.replace( - "export const VideoApiAdapterMap = {", - "export const VideoApiAdapterMap = process.env.NEXT_PUBLIC_IS_E2E === '1' ? {} : {" - ), + exportLine.replace(`export const VideoApiAdapterMap = {`, gateExportHeader("VideoApiAdapterMap")),Also applies to: 417-418, 469-470
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.husky/pre-commit(1 hunks)packages/app-store-cli/src/build.ts(3 hunks)packages/app-store/analytics.services.generated.ts(1 hunks)packages/app-store/calendar.services.generated.ts(1 hunks)packages/app-store/video.adapters.generated.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/app-store-cli/src/build.tspackages/app-store/video.adapters.generated.tspackages/app-store/analytics.services.generated.tspackages/app-store/calendar.services.generated.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/app-store-cli/src/build.tspackages/app-store/video.adapters.generated.tspackages/app-store/analytics.services.generated.tspackages/app-store/calendar.services.generated.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/app-store-cli/src/build.tspackages/app-store/video.adapters.generated.tspackages/app-store/analytics.services.generated.tspackages/app-store/calendar.services.generated.ts
🧠 Learnings (3)
📚 Learning: 2025-09-02T12:29:32.592Z
Learnt from: CR
PR: calcom/cal.com#0
File: AGENTS.md:0-0
Timestamp: 2025-09-02T12:29:32.592Z
Learning: Use `yarn build` to build all packages
Applied to files:
.husky/pre-commit
📚 Learning: 2025-08-30T00:30:44.247Z
Learnt from: saurabhraghuvanshii
PR: calcom/cal.com#23460
File: packages/app-store/paypal/components/PaypalPaymentComponent.tsx:55-55
Timestamp: 2025-08-30T00:30:44.247Z
Learning: Cal.com uses a build-time script `apps/web/scripts/copy-app-store-static.js` that automatically copies static assets from `packages/app-store/**/static/**/*` to `apps/web/public/app-store/` during both development and build processes.
Applied to files:
.husky/pre-commit
📚 Learning: 2025-08-05T12:04:29.037Z
Learnt from: din-prajapati
PR: calcom/cal.com#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.
Applied to files:
packages/app-store/calendar.services.generated.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Detect changes
🔇 Additional comments (4)
.husky/pre-commit (1)
6-7: Do not add an explicit generator call — the build already runs the generatorRoot package script runs
yarn turbo build --filter=@calcom/app-store-cliand packages/app-store-cli'sbuild/generatescripts executets-node --transpile-only src/build.ts(the generator); packages/app-store/.generated. were updated on 2025-09-12, so addingnode packages/app-store-cli/dist/build.jsis unnecessary.Likely an incorrect or invalid review comment.
packages/app-store/analytics.services.generated.ts (1)
5-10: LGTM: strict E2E gate fixes false positives in Docker.Explicit
process.env.NEXT_PUBLIC_IS_E2E === "1"avoids accidental disabling when set to arbitrary non-empty strings.packages/app-store/video.adapters.generated.ts (1)
5-20: LGTM: adapters now load outside explicit E2E.The tightened check restores dynamic imports (e.g., nextcloudtalk, zoomvideo) in self-hosted/prod.
packages/app-store/calendar.services.generated.ts (1)
5-20: LGTM: calendar integrations are no longer gated by any truthy value.This should re-enable Google/Outlook/etc. in Docker unless
NEXT_PUBLIC_IS_E2E="1".
.husky/pre-commit
Outdated
| npx turbo build --filter=@calcom/app-store-cli | ||
| git add packages/app-store/*.generated.* |
There was a problem hiding this comment.
Can you explain this change? I don't think it's needed here. Others look good
|
it was failing with Couldn't find a script named turbo' error whenever someone tried to commit changes. I fixed it by using |
github-actions
bot
added
Low priority
E2E results are ready! |
- Remove unused isCalendarService function - Fix any type usage in getCalendar - Add eslint-disable for necessary app-store imports in videoClient - Maintain architectural separation while fixing core functionality
Head branch was pushed to by a user without write access
nehaaprasad
force-pushed
the
fix/dyn-video
branch
from
96e3deb to
609ad74
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/lib/videoClient.ts (3)
97-114: Bug: success=true even when no adapter or createMeeting returns undefined.If firstVideoAdapter is missing, or returns undefined, this path still marks success true. Force failure to trigger fallback.
if (!enabledApp?.enabled) throw `Location app ${credential.appId} is either disabled or not seeded at all`; - createdMeeting = await firstVideoAdapter?.createMeeting(calEvent); - - returnObject = { ...returnObject, createdEvent: createdMeeting, success: true }; + if (!firstVideoAdapter) { + throw new Error(`No video adapter available for type ${credential.type}`); + } + createdMeeting = await firstVideoAdapter.createMeeting(calEvent); + if (!createdMeeting) { + throw new Error(`Video adapter ${credential.appId} returned no meeting`); + } + returnObject = { ...returnObject, createdEvent: createdMeeting, success: true };
145-147: Sanitize logs: avoid PII leakage in updateMeeting error path.Log PII-free event data, consistent with createMeeting.
- log.error("updateMeeting failed", e, calEvent); + log.error( + "updateMeeting failed", + safeStringify(e), + safeStringify({ calEvent: getPiiFreeCalendarEvent(calEvent) }) + );
140-151: Require an adapter and fail the update path when updateMeeting didn't run/succeedInclude adapter presence in canCallUpdateMeeting and force success = false when updatedMeeting is falsy.
- const [firstVideoAdapter] = await getVideoAdapters([credential]); - const canCallUpdateMeeting = !!(credential && bookingRef); + const [firstVideoAdapter] = await getVideoAdapters([credential]); + const canCallUpdateMeeting = !!(credential && bookingRef && firstVideoAdapter); @@ - if (!updatedMeeting) { + if (!updatedMeeting) { @@ - return { + return { @@ - success, + success: false,Add an E2E test (E2E="1") where the adapter map is empty and assert updateMeeting returns success:false.
Also applies to: packages/lib/videoClient.ts lines 152-164.
🧹 Nitpick comments (3)
packages/lib/videoClient.ts (3)
4-9: Avoid bypassing no-restricted-imports; prefer a safe re-export boundary.Importing from app-store in lib breaks layering. Create a shared, allowed module (e.g., @calcom/video-shared) that re-exports DailyLocationType and getDailyAppKeys, or whitelist this file via ESLint config instead of inline disables.
31-36: Tighten typing on map access.Remove the second cast by casting the derived key once; improves readability and type safety.
- const appName = deriveAppDictKeyFromType(cred.type, VideoApiAdapterMap); - const videoAdapterImport = VideoApiAdapterMap[appName as keyof typeof VideoApiAdapterMap]; + const appName = deriveAppDictKeyFromType(cred.type, VideoApiAdapterMap) as keyof typeof VideoApiAdapterMap; + const videoAdapterImport = VideoApiAdapterMap[appName];Consider adding unit cases for tricky types (e.g., "nextcloud_talk_video", "office365_video", "webex_video") to ensure the derived key matches VideoApiAdapterMap keys.
37-41: Downgrade log severity under E2E gating to avoid noisy errors.When NEXT_PUBLIC_IS_E2E === "1", the map is empty by design. Use debug in that context.
- if (!videoAdapterImport) { - log.error(`Couldn't get adapter for ${appName}`); + if (!videoAdapterImport) { + const isE2E = process.env.NEXT_PUBLIC_IS_E2E === "1"; + (isE2E ? log.debug : log.error)(`Couldn't get adapter for ${String(appName)}`); continue; }Please confirm CI/E2E sets NEXT_PUBLIC_IS_E2E="1" (string), not "true".
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.husky/pre-commit(1 hunks)packages/app-store/_utils/getCalendar.ts(3 hunks)packages/lib/videoClient.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .husky/pre-commit
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/app-store/_utils/getCalendar.tspackages/lib/videoClient.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/app-store/_utils/getCalendar.tspackages/lib/videoClient.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/app-store/_utils/getCalendar.tspackages/lib/videoClient.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/config.json:5-5
Timestamp: 2025-09-01T07:31:00.963Z
Learning: In Cal.com's video integration system, credential types (e.g., "kyzon-space_video") are transformed to app names by removing underscores using `cred.type.split("_").join("")` in videoClient.ts line 28. This means the key in packages/app-store/index.ts should match the underscore-removed version (e.g., "kyzon-spacevideo") rather than the original type name. This same pattern is used in other parts of the system like getCalendar.ts.
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/config.json:5-5
Timestamp: 2025-09-01T07:31:00.963Z
Learning: In Cal.com's video integration system, credential types (e.g., "kyzon-space_video") are transformed to app names by removing underscores using `cred.type.split("_").join("")` in videoClient.ts line 28. This means the key in packages/app-store/index.ts should match the underscore-removed version (e.g., "kyzon-spacevideo") rather than the original type name.
📚 Learning: 2025-09-01T07:31:00.963Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/config.json:5-5
Timestamp: 2025-09-01T07:31:00.963Z
Learning: In Cal.com's video integration system, credential types (e.g., "kyzon-space_video") are transformed to app names by removing underscores using `cred.type.split("_").join("")` in videoClient.ts line 28. This means the key in packages/app-store/index.ts should match the underscore-removed version (e.g., "kyzon-spacevideo") rather than the original type name. This same pattern is used in other parts of the system like getCalendar.ts.
Applied to files:
packages/app-store/_utils/getCalendar.tspackages/lib/videoClient.ts
📚 Learning: 2025-08-05T12:04:29.037Z
Learnt from: din-prajapati
PR: calcom/cal.com#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.
Applied to files:
packages/app-store/_utils/getCalendar.ts
📚 Learning: 2025-09-01T07:31:00.963Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/config.json:5-5
Timestamp: 2025-09-01T07:31:00.963Z
Learning: In Cal.com's video integration system, credential types (e.g., "kyzon-space_video") are transformed to app names by removing underscores using `cred.type.split("_").join("")` in videoClient.ts line 28. This means the key in packages/app-store/index.ts should match the underscore-removed version (e.g., "kyzon-spacevideo") rather than the original type name.
Applied to files:
packages/app-store/_utils/getCalendar.tspackages/lib/videoClient.ts
🧬 Code graph analysis (2)
packages/app-store/_utils/getCalendar.ts (1)
packages/app-store/calendar.services.generated.ts (1)
CalendarServiceMap(5-20)
packages/lib/videoClient.ts (1)
packages/app-store/video.adapters.generated.ts (1)
VideoApiAdapterMap(5-20)
🔇 Additional comments (4)
packages/lib/videoClient.ts (1)
12-12: Good move: centralized key derivation.Switching to deriveAppDictKeyFromType aligns lookups with generated map keys and prevents drift.
packages/app-store/_utils/getCalendar.ts (3)
1-1: Good change: unify key derivation.Using deriveAppDictKeyFromType keeps lookups consistent with generated map keys.
23-25: LGTM; ensure edge-case mappings are covered by tests.Please add/confirm cases like "ics_feed_calendar" → "ics-feedcalendar" and “exchange2016_calendar” → “exchange2016calendar”.
40-40: Constructor typing: verify all CalendarService implementations accept CredentialForCalendarService.Previously this was cast; passing the typed credential is preferable but may surface type mismatches in some services.
Run a typecheck locally to confirm no CalendarService constructors require a different shape.
2 participants
fixes: #23760
Root Cause
The issue was in three generated service maps that were being disabled in production environments:
VideoApiAdapterMap- for video integrations (NextCloud Talk, Zoom, etc.)CalendarServiceMap- for calendar integrations (Google, Outlook, etc.)AnalyticsServiceMap- for analytics servicesThe condition
process.env.NEXT_PUBLIC_IS_E2Ewas evaluating as truthy in Docker environments (when set to"true"or other values), causing all integrations to be disabled instead of only during E2E tests.Changes Made
process.env.NEXT_PUBLIC_IS_E2Etoprocess.env.NEXT_PUBLIC_IS_E2E === "1"npx turboinstead of yarnBefore Fix:
After Fix:
Files Changed
packages/app-store/video.adapters.generated.tspackages/app-store/calendar.services.generated.tspackages/app-store/analytics.services.generated.tspackages/app-store-cli/src/build.ts.husky/pre-commit