Conversation
|
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughAdds a new "kyzonspacevideo" app integration to the app-store. Registers its metadata and Zod schemas in generated maps, adds an API handler and VideoApiAdapter mapping, and introduces a new package at packages/app-store/kyzonspacevideo containing config.json, DESCRIPTION.md, Zod schemas, OAuth add/callback API routes and tests, Axios client, token manager and tests, credential key utilities, API type definitions, a VideoApiAdapter implementation and exports via index files and package.json. Possibly related PRs
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
@nangelina is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
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: 13
🧹 Nitpick comments (21)
packages/app-store/kyzon-space/DESCRIPTION.md (1)
1-7: Add alt text for screenshots (if schema supports) and verify assets existImprove accessibility and avoid broken images by adding alt text per item and confirming 1.png–4.png exist in this folder.
packages/app-store/kyzon-space/lib/axios.ts (1)
5-7: Optional: centralized retries and 401 refreshIf not already in token manager/interceptors, add exponential backoff for 429/5xx and refresh on 401 to reduce flaky failures.
packages/app-store/kyzon-space/config.json (1)
7-7: Consider adding policy URLsIf supported by the manifest, include
privacyPolicyUrlandtermsUrlfor the app listing.packages/app-store/kyzon-space/package.json (1)
7-12: Pin internal deps or use workspace protocol for stability."" for @calcom/ can mask breaking changes. Prefer "workspace:^" (or pin to a known range) to keep CI deterministic.
"dependencies": { - "@calcom/lib": "*" + "@calcom/lib": "workspace:^" }, "devDependencies": { - "@calcom/types": "*" + "@calcom/types": "workspace:^" },packages/app-store/kyzon-space/lib/index.ts (1)
1-1: Prefer named exports over default exports.To align with repo guidance, convert VideoApiAdapter’s default export to a named export and re-export it here.
-export { default as VideoApiAdapter } from "./VideoApiAdapter"; +export { VideoApiAdapter } from "./VideoApiAdapter";Outside this file (for completeness), adjust VideoApiAdapter.ts:
// before export default class VideoApiAdapter { /* ... */ } // after export class VideoApiAdapter { /* ... */ }packages/app-store/kyzon-space/api/index.ts (1)
1-2: Prefer named exports over re-exporting defaults.This keeps us aligned with our guideline to avoid default exports in TS/JS modules and simplifies refactors.
Apply in this file:
-export { default as add } from "./add"; -export { default as callback } from "./callback"; +export { add } from "./add"; +export { callback } from "./callback";And adjust implementations accordingly (outside this diff):
// packages/app-store/kyzon-space/api/add.ts export const add = async (req, res) => { /* ... */ }; // packages/app-store/kyzon-space/kyzon-space/api/callback.ts export const callback = async (req, res) => { /* ... */ };I can open a follow-up PR to migrate these safely if you prefer.
packages/app-store/kyzon-space/lib/getKyzonAppKeys.ts (1)
1-8: Mark this module server-only and add explicit return type+import "server-only"; +import type { z } from "zod"; import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug"; import config from "../config.json"; import { appKeysSchema as kyzonSpaceAppKeysSchema } from "../zod"; +type KyzonSpaceAppKeys = z.infer<typeof kyzonSpaceAppKeysSchema>; -export const getKyzonAppKeys = async () => { +export const getKyzonAppKeys = async (): Promise<KyzonSpaceAppKeys> => { const appKeys = await getAppKeysFromSlug(config.slug); return kyzonSpaceAppKeysSchema.parse(appKeys); };Ensures this secret-fetching code never ends up in client bundles and enforces the parsed type at compile time.
packages/app-store/kyzon-space/zod.ts (1)
5-9: Trim credentials to avoid accidental whitespace in secretsAdd
.trim()to enforce non-empty, whitespace-stripped strings.export const appKeysSchema = z.object({ - client_id: z.string().min(1), - client_secret: z.string().min(1), - api_key: z.string().min(1), + client_id: z.string().trim().min(1), + client_secret: z.string().trim().min(1), + api_key: z.string().trim().min(1), });packages/app-store/kyzon-space/api/add.ts (1)
2-3: Prefer URLSearchParams over querystring for OAuth URL buildingSlight simplification and avoids legacy
querystring.-import { stringify } from "querystring"; +// no need for querystring; use URLSearchParams @@ - const params = { - response_type: "code", - client_id, - redirect_uri: `${WEBAPP_URL}/api/integrations/${config.slug}/callback`, - scope: "meetings:write calendar:write profile:read", - state, - }; - const query = stringify(params); - const url = `${kyzonBaseUrl}/oauth/authorize?${query}`; + const params = new URLSearchParams({ + response_type: "code", + client_id, + redirect_uri: `${WEBAPP_URL}/api/integrations/${config.slug}/callback`, + scope: "meetings:write calendar:write profile:read", + state, + }); + const url = `${kyzonBaseUrl}/oauth/authorize?${params.toString()}`;Also applies to: 23-31
packages/app-store/kyzon-space/lib/tokenManager.ts (3)
13-15: Select only needed fields from PrismaAdhere to the repo guideline: select only required columns.
- const credential = await prisma.credential.findUnique({ - where: { id: credentialId }, - }); + const credential = await prisma.credential.findUnique({ + where: { id: credentialId }, + select: { id: true, key: true }, + });
40-42: Header casing inconsistency with callback handlerUnify to "X-API-Key" to match the callback route and reduce confusion in tracing.
- "X-Api-Key": api_key, + "X-API-Key": api_key,
1-70: Set an HTTP timeout on KYZON Axios instanceToken refresh should fail fast to avoid hanging API requests. Consider adding a reasonable timeout (e.g., 10s) in
lib/axios.tsand optional retry with jitter for transient 5xx.packages/app-store/kyzon-space/api/callback.ts (2)
4-15: Use a single WEBAPP_URL source for redirect_uri (consistent with add.ts)Unify to
WEBAPP_URLto avoid env drift between routes.+import { WEBAPP_URL } from "@calcom/lib/constants"; @@ - redirect_uri: `${process.env.NEXT_PUBLIC_WEBAPP_URL}/api/integrations/${config.slug}/callback`, + redirect_uri: `${WEBAPP_URL}/api/integrations/${config.slug}/callback`,Also applies to: 71-72
144-146: Consider awaiting default app set before redirectIf
setDefaultConferencingAppis async, the redirect can race the preference write.- if (state?.defaultInstall) { - setDefaultConferencingApp(userId, config.slug); - } + if (state?.defaultInstall) { + await setDefaultConferencingApp(userId, config.slug); + }Can you confirm whether
setDefaultConferencingAppreturns a Promise in this codebase?packages/app-store/kyzon-space/lib/VideoApiAdapter.ts (6)
68-71: Avoid empty thirdPartySource.eventIdDon’t send empty strings; omit the object when uid is absent (shown in diff above).
Also applies to: 114-116
143-157: Rename param for clarity: uid → meetingIdThis endpoint deletes a calendar-event by ID; “uid” is ambiguous in Cal.com.
Apply:
- deleteMeeting: async (uid: string): Promise<void> => { + deleteMeeting: async (meetingId: string): Promise<void> => { @@ - await kyzonAxiosInstance.delete(`/v1/teams/${key.team_id}/calendar-events/${uid}`, { + await kyzonAxiosInstance.delete(`/v1/teams/${key.team_id}/calendar-events/${meetingId}`, { @@ - console.warn(`Failed to delete KYZON calendar event ${uid}:`, error); + console.warn(`Failed to delete KYZON calendar event ${meetingId}:`, error);
182-195: Availability: confirm treatment of all-day/ongoing eventsYou currently drop all-day/ongoing. Product-wise, these often block scheduling. Consider marking them as full-day busy or configurable.
196-200: Returning [] on error can show false availabilityPrefer surfacing an error up the stack or at least emitting structured telemetry for observability.
204-204: Prefer named export over default exportAligns with repo guideline on named exports.
Apply:
-export default KyzonVideoApiAdapter; +export { KyzonVideoApiAdapter };And update imports accordingly.
36-87: Test coverage suggestions (happy path + edge cases)
- createMeeting: success, calendar failure cleanup, missing uid (no thirdPartySource).
- Token refresh: 401 once then succeeds.
- updateMeeting: no meetingId -> create; with meetingId -> update; update failure -> fallback.
- deleteMeeting: 404 ignored; authorization failure surfaces.
- getAvailability: range mapping, all-day behavior, error path telemetry.
I can scaffold tests with axios-mock-adapter and zod-safe fixtures.
packages/app-store/kyzon-space/lib/apiTypes.ts (1)
44-49: Use standard MakeRequired helper for clarityThe Omit + Required + Pick pattern reads clearer and avoids accidental optional intersections.
Apply:
-type MakeRequired<T, K extends keyof T> = T & { [P in K]-?: T[P] }; +type MakeRequired<T, K extends keyof T> = Omit<T, K> & Required<Pick<T, K>>;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (6)
packages/app-store/kyzon-space/static/1.pngis excluded by!**/*.pngpackages/app-store/kyzon-space/static/2.pngis excluded by!**/*.pngpackages/app-store/kyzon-space/static/3.pngis excluded by!**/*.pngpackages/app-store/kyzon-space/static/4.pngis excluded by!**/*.pngpackages/app-store/kyzon-space/static/icon.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
packages/app-store/apps.keys-schemas.generated.ts(2 hunks)packages/app-store/apps.metadata.generated.ts(2 hunks)packages/app-store/apps.schemas.generated.ts(2 hunks)packages/app-store/apps.server.generated.ts(1 hunks)packages/app-store/bookerApps.metadata.generated.ts(2 hunks)packages/app-store/index.ts(1 hunks)packages/app-store/kyzon-space/DESCRIPTION.md(1 hunks)packages/app-store/kyzon-space/api/add.ts(1 hunks)packages/app-store/kyzon-space/api/callback.ts(1 hunks)packages/app-store/kyzon-space/api/index.ts(1 hunks)packages/app-store/kyzon-space/config.json(1 hunks)packages/app-store/kyzon-space/index.ts(1 hunks)packages/app-store/kyzon-space/lib/KyzonCredentialKey.ts(1 hunks)packages/app-store/kyzon-space/lib/VideoApiAdapter.ts(1 hunks)packages/app-store/kyzon-space/lib/apiTypes.ts(1 hunks)packages/app-store/kyzon-space/lib/axios.ts(1 hunks)packages/app-store/kyzon-space/lib/getKyzonAppKeys.ts(1 hunks)packages/app-store/kyzon-space/lib/index.ts(1 hunks)packages/app-store/kyzon-space/lib/tokenManager.ts(1 hunks)packages/app-store/kyzon-space/package.json(1 hunks)packages/app-store/kyzon-space/zod.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/index.tspackages/app-store/kyzon-space/api/index.tspackages/app-store/apps.server.generated.tspackages/app-store/kyzon-space/zod.tspackages/app-store/apps.metadata.generated.tspackages/app-store/kyzon-space/index.tspackages/app-store/kyzon-space/lib/index.tspackages/app-store/bookerApps.metadata.generated.tspackages/app-store/apps.schemas.generated.tspackages/app-store/kyzon-space/lib/getKyzonAppKeys.tspackages/app-store/kyzon-space/lib/tokenManager.tspackages/app-store/kyzon-space/lib/axios.tspackages/app-store/kyzon-space/api/callback.tspackages/app-store/kyzon-space/lib/VideoApiAdapter.tspackages/app-store/kyzon-space/lib/KyzonCredentialKey.tspackages/app-store/kyzon-space/api/add.tspackages/app-store/apps.keys-schemas.generated.tspackages/app-store/kyzon-space/lib/apiTypes.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/index.tspackages/app-store/kyzon-space/api/index.tspackages/app-store/apps.server.generated.tspackages/app-store/kyzon-space/zod.tspackages/app-store/apps.metadata.generated.tspackages/app-store/kyzon-space/index.tspackages/app-store/kyzon-space/lib/index.tspackages/app-store/bookerApps.metadata.generated.tspackages/app-store/apps.schemas.generated.tspackages/app-store/kyzon-space/lib/getKyzonAppKeys.tspackages/app-store/kyzon-space/lib/tokenManager.tspackages/app-store/kyzon-space/lib/axios.tspackages/app-store/kyzon-space/api/callback.tspackages/app-store/kyzon-space/lib/VideoApiAdapter.tspackages/app-store/kyzon-space/lib/KyzonCredentialKey.tspackages/app-store/kyzon-space/api/add.tspackages/app-store/apps.keys-schemas.generated.tspackages/app-store/kyzon-space/lib/apiTypes.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/index.tspackages/app-store/kyzon-space/api/index.tspackages/app-store/apps.server.generated.tspackages/app-store/kyzon-space/zod.tspackages/app-store/apps.metadata.generated.tspackages/app-store/kyzon-space/index.tspackages/app-store/kyzon-space/lib/index.tspackages/app-store/bookerApps.metadata.generated.tspackages/app-store/apps.schemas.generated.tspackages/app-store/kyzon-space/lib/getKyzonAppKeys.tspackages/app-store/kyzon-space/lib/tokenManager.tspackages/app-store/kyzon-space/lib/axios.tspackages/app-store/kyzon-space/api/callback.tspackages/app-store/kyzon-space/lib/VideoApiAdapter.tspackages/app-store/kyzon-space/lib/KyzonCredentialKey.tspackages/app-store/kyzon-space/api/add.tspackages/app-store/apps.keys-schemas.generated.tspackages/app-store/kyzon-space/lib/apiTypes.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.156Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
🧬 Code graph analysis (4)
packages/app-store/kyzon-space/lib/tokenManager.ts (3)
packages/app-store/kyzon-space/lib/KyzonCredentialKey.ts (3)
KyzonCredentialKey(13-13)kyzonCredentialKeySchema(3-11)getKyzonCredentialKey(15-30)packages/app-store/kyzon-space/lib/getKyzonAppKeys.ts (1)
getKyzonAppKeys(5-8)packages/app-store/kyzon-space/lib/axios.ts (1)
kyzonAxiosInstance(5-7)
packages/app-store/kyzon-space/api/callback.ts (3)
packages/app-store/kyzon-space/lib/getKyzonAppKeys.ts (1)
getKyzonAppKeys(5-8)packages/app-store/kyzon-space/lib/KyzonCredentialKey.ts (2)
KyzonCredentialKey(13-13)getKyzonCredentialKey(15-30)packages/app-store/kyzon-space/lib/axios.ts (1)
kyzonAxiosInstance(5-7)
packages/app-store/kyzon-space/lib/VideoApiAdapter.ts (6)
packages/types/VideoApiAdapter.d.ts (1)
VideoApiAdapter(19-49)packages/app-store/kyzon-space/lib/KyzonCredentialKey.ts (2)
KyzonCredentialKey(13-13)kyzonCredentialKeySchema(3-11)packages/app-store/kyzon-space/lib/tokenManager.ts (2)
isTokenExpired(64-69)refreshKyzonToken(11-61)packages/types/Calendar.d.ts (2)
CalendarEvent(163-226)RecurringEvent(129-136)packages/app-store/kyzon-space/lib/axios.ts (1)
kyzonAxiosInstance(5-7)packages/app-store/kyzon-space/lib/apiTypes.ts (7)
KyzonSpaceCallResponse(6-11)KyzonCreateSpaceCallRequestBody(1-4)KyzonGetCalendarEventResponse(46-59)KyzonCreateOrPutCalendarEventRequestBody(20-42)KyzonSingleSpaceCallWithinRangeResponse(66-93)KyzonGetSpaceCallsWithinRangeRequestQuery(61-64)KyzonCalendarEventRecurrence(13-18)
packages/app-store/kyzon-space/api/add.ts (6)
packages/app-store/kyzon-space/api/callback.ts (1)
handler(16-151)packages/app-store/kyzon-space/lib/getKyzonAppKeys.ts (1)
getKyzonAppKeys(5-8)packages/lib/constants.ts (1)
WEBAPP_URL(12-18)packages/app-store/kyzon-space/lib/axios.ts (1)
kyzonBaseUrl(3-3)packages/lib/server/defaultHandler.ts (1)
defaultHandler(8-24)packages/lib/server/defaultResponder.ts (1)
defaultResponder(11-42)
⏰ 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). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (16)
packages/app-store/kyzon-space/lib/axios.ts (1)
5-7: Use a dedicated base URL for OAuth endpoints
kyzonBaseUrlis set tohttps://kyzonsolutions.com/api/cloud, so both/oauth/authorize(in add.ts) and/oauth/token(in callback.ts and tokenManager.ts) resolve under/api/cloud/oauth. If your OAuth server isn’t hosted there, introduce and import a separateauthBaseUrl(e.g. from config.json or env) and use that for all/oauth/*requests.packages/app-store/kyzon-space/config.json (1)
19-23: Description OK; keep copy in DESCRIPTION.md in syncConfig description looks fine. After updating DESCRIPTION.md, ensure both surfaces say the same thing.
packages/app-store/apps.metadata.generated.ts (2)
51-51: LGTM: kyzon-space config imported
160-160: LGTM: appStoreMetadata maps "kyzon-space"packages/app-store/bookerApps.metadata.generated.ts (2)
22-22: LGTM: kyzon-space config imported for booker apps
68-68: LGTM: booker appStoreMetadata maps "kyzon-space"packages/app-store/kyzon-space/package.json (1)
6-6: Resolve TS entry consistency
The"main": "./index.ts"setting aligns with all otherpackages/app-store/*/package.jsonentries (none include"type","exports", or"types"fields), so no changes are required.packages/app-store/kyzon-space/lib/KyzonCredentialKey.ts (1)
3-30: Exclude credential.key from all API and tRPC outputs. Audit every Next.js API route and tRPC procedure to confirm you never serialize or return the raw credential.key—use explicit omit/select logic or define output schemas that strip this field.packages/app-store/apps.keys-schemas.generated.ts (1)
24-24: Keys schema mapping added correctly.The import and "kyzon-space" entry look good and consistent with app data schemas.
Also applies to: 76-76
packages/app-store/kyzon-space/index.ts (1)
1-2: LGTM — clear package surface with named namespaces.No concerns. This aligns with our “no default export” preference at package entry points.
packages/app-store/apps.server.generated.ts (1)
51-51: Slug alignment and API imports verified; no changes needed.packages/app-store/index.ts (1)
15-15: Confirm hyphen in app-store key 'kyzon-spacevideo' (add alias or rename).packages/app-store/index.ts currently exports "kyzon-spacevideo" (≈line 15); runtime lookup in packages/lib/videoClient.ts derives app names by removing underscores (e.g. "zoom_video" → "zoomvideo"), so credential types that resolve to "kyzonspacevideo" will not match the hyphenated key.
- Action: either rename the key to kyzonspacevideo or add an alias next to it:
kyzonspacevideo: createCachedImport(() => import("./kyzon-space")).Locations: packages/app-store/index.ts:15; packages/lib/videoClient.ts:28.
packages/app-store/kyzon-space/zod.ts (1)
5-9: Confirm naming: api_key vs api_tokenPR text mentions “api_token” for testing creds, while schema expects “api_key”. Please confirm the canonical name across docs, secrets storage, and UI so generated schemas and
getKyzonAppKeys()align.packages/app-store/kyzon-space/lib/VideoApiAdapter.ts (2)
61-64: Ensure UTC strings for KYZON’s UTC fieldsUse toISOString to guarantee UTC. Avoid sending local/timezone-formatted strings.
Already included in the diff above for createMeeting; mirror the same change in updateMeeting:
- startDateUtcISOString: event.startTime, - endDateUtcISOString: event.endTime, + startDateUtcISOString: new Date(event.startTime).toISOString(), + endDateUtcISOString: new Date(event.endTime).toISOString(),Also applies to: 106-109
39-51: Verify Space Call DELETE endpoint
I didn’t find any existing implementation of a space-call deletion in VideoApiAdapter; please confirm that the KYZON API supports DELETE/v1/teams/{team_id}/space/calls/{spaceCallId}(path and semantics) before merging.packages/app-store/kyzon-space/lib/apiTypes.ts (1)
61-93: Type shapes look consistent with adapter usageInterfaces align with adapter calls and reduce runtime casting.
| import { appDataSchema as intercom_zod_ts } from "./intercom/zod"; | ||
| import { appDataSchema as jelly_zod_ts } from "./jelly/zod"; | ||
| import { appDataSchema as jitsivideo_zod_ts } from "./jitsivideo/zod"; | ||
| import { appDataSchema as kyzon_space_zod_ts } from "./kyzon-space/zod"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Autogen delta looks correct; ensure generator artifacts are in sync.
The new "kyzon-space" import and mapping entry align with the integration. Re-run the builder in case of local cache drift.
Also applies to: 76-76
🏁 Script executed:
#!/bin/bash
yarn app-store:build
git diff --name-only -- packages/app-store | catLength of output: 807
Fix build failure: fastqueue concurrency must be greater than 1
The yarn app-store:build step is failing with an internal “fastqueue concurrency must be greater than 1” error. Resolve this build error (e.g. adjust fastqueue/Yarn configuration or upgrade dependencies) so the build completes successfully.
…_conferencing to kyzon-space_video due to the delete meeting api only being called for types ending with _video.
nangelina
force-pushed
the
angelina/kyzon-space-app
branch
from
cfc7429 to
db81e14
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/app-store/kyzon-space/lib/tokenManager.ts (2)
21-22: Guard missingrefresh_tokenbefore calling provider
refresh_tokenis optional in the schema; avoid 400s and noisy logs by short‑circuiting.const currentKey = kyzonCredentialKeySchema.parse(credential.key); + if (!currentKey.refresh_token) { + console.warn(`KYZON refresh skipped: credential ${credentialId} has no refresh_token`); + return null; + }
57-60: Sanitize error logs; avoid dumping secrets from Axios errorsDon’t log the entire error object; Axios may include
config.datawith tokens.- } catch (error) { - console.error("Failed to refresh KYZON token:", error); + } catch (error) { + const err = error as any; + console.error("Failed to refresh KYZON token", { + status: err?.response?.status, + message: err?.message, + code: err?.code, + }); return null; }packages/app-store/kyzon-space/api/add.ts (1)
1-1: Return proper HTTP 401 for unauthenticated requests (use NextApiResponse and set status).Right now unauthorized requests still end up as HTTP 200 because defaultResponder json-serializes your return value. Set the status on the response and return a body object (don’t call res.json here) so defaultResponder emits the JSON with the 401 already set.
-import type { NextApiRequest } from "next"; +import type { NextApiRequest, NextApiResponse } from "next"; -async function handler(req: NextApiRequest) { +async function handler(req: NextApiRequest, res: NextApiResponse) { // Get user const user = req?.session?.user; if (!user) { - return { status: 401, body: { error: "Unauthorized" } }; + res.status(401); + return { message: "Unauthorized" }; }If you prefer to call res.status(...).json(...) directly, drop defaultResponder and wire the handler directly via defaultHandler to avoid double responses. I can provide that variant if you want.
Also applies to: 13-18
🧹 Nitpick comments (9)
packages/app-store/kyzon-space/lib/tokenManager.ts (5)
13-15: Use Prismaselectto fetch only needed fieldsFollow our guideline to minimize fetched columns; you only need
keyhere.- const credential = await prisma.credential.findUnique({ - where: { id: credentialId }, - }); + const credential = await prisma.credential.findUnique({ + where: { id: credentialId }, + select: { key: true }, + });
25-44: Confirm content type; token endpoints usually require form-encoded bodyOAuth 2.0 token endpoints commonly expect
application/x-www-form-urlencodedrather than JSON. If KYZON follows RFC 6749, switch to form-encoded; otherwise, keep JSON.- const { data: newTokens } = await kyzonAxiosInstance.post<{ - access_token: string; - refresh_token: string; - token_type: "Bearer"; - expires_in: number; - scope: string; - }>( - "/oauth/token", - { - grant_type: "refresh_token", - refresh_token: currentKey.refresh_token, - client_id, - client_secret, - }, - { - headers: { - "X-Api-Key": api_key, - }, - } - ); + type KyzonRefreshResponse = { + access_token: string; + token_type: "Bearer"; + expires_in: number; + scope?: string; + refresh_token?: string; + }; + const body = new URLSearchParams({ + grant_type: "refresh_token", + refresh_token: currentKey.refresh_token!, + client_id, + client_secret, + }); + const { data: newTokens } = await kyzonAxiosInstance.post<KyzonRefreshResponse>("/oauth/token", body, { + headers: { + "X-Api-Key": api_key, + "Content-Type": "application/x-www-form-urlencoded", + }, + });
25-31: Makerefresh_tokenoptional in the response type and preserve the old tokenSome providers omit
refresh_tokenon refresh; ensure we don’t null it out and fix the type.- const { data: newTokens } = await kyzonAxiosInstance.post<{ - access_token: string; - refresh_token: string; - token_type: "Bearer"; - expires_in: number; - scope: string; - }>( + type KyzonRefreshResponse = { + access_token: string; + token_type: "Bearer"; + expires_in: number; + scope?: string; + refresh_token?: string; + }; + const { data: newTokens } = await kyzonAxiosInstance.post<KyzonRefreshResponse>( ... - const newCredentialKey = getKyzonCredentialKey({ ...currentKey, ...newTokens }); + const merged = { + ...currentKey, + ...newTokens, + refresh_token: newTokens.refresh_token ?? currentKey.refresh_token, + }; + const newCredentialKey = getKyzonCredentialKey(merged);Also applies to: 46-46
63-69: Harden expiry check against malformed tokensFail safe to “expired” if
expiry_dateis missing or invalid.export function isTokenExpired(token: KyzonCredentialKey): boolean { - const now = Date.now(); - const bufferTime = 5 * 60 * 1000; - - return token.expiry_date - bufferTime <= now; + const now = Date.now(); + const bufferTime = 5 * 60 * 1000; + if (!token || typeof token.expiry_date !== "number" || !Number.isFinite(token.expiry_date)) { + return true; + } + return token.expiry_date - bufferTime <= now; }
48-55: Consider concurrent refresh racesIf multiple requests refresh the same credential concurrently, last-write-wins can thrash tokens. Consider a per-credential mutex or a lightweight DB lock to serialize refreshes.
Do you want a small Redis/memory-mutex utility compatible with the existing app store pattern?
packages/app-store/kyzon-space/api/add.ts (4)
2-2: Use URLSearchParams instead of legacy querystring.stringify.querystring is legacy; URLSearchParams handles RFC 3986 encoding more predictably for OAuth params.
-import { stringify } from "querystring"; @@ - const params = { - response_type: "code", - client_id, - redirect_uri: `${WEBAPP_URL}/api/integrations/${config.slug}/callback`, - scope: "meetings:write calendar:write profile:read", - state, - }; - const query = stringify(params); - const url = `${kyzonBaseUrl}/oauth/authorize?${query}`; + const params = new URLSearchParams({ + response_type: "code", + client_id, + redirect_uri: `${WEBAPP_URL}/api/integrations/${config.slug}/callback`, + scope: "meetings:write calendar:write profile:read`, + state, + }); + const url = `${kyzonBaseUrl}/oauth/authorize?${params.toString()}`; ``` <!-- review_comment_end --> Also applies to: 23-31 --- `20-21`: **Defensive handling when KYZON app keys are missing/misconfigured.** Fail fast with a 500 and actionable message instead of an unhandled exception. ```diff - const { client_id } = await getKyzonAppKeys(); + const keys = await getKyzonAppKeys().catch(() => null); + if (!keys?.client_id) { + res.status(500); + return { message: "KYZON app keys are missing or misconfigured" }; + } + const { client_id } = keys; ``` <!-- review_comment_end --> --- `36-38`: **Export the handler as a named export for testability while keeping the default export.** Small ergonomic win for unit tests and reuse; no behavioral change. ```diff -async function handler(req: NextApiRequest, res: NextApiResponse) { +export async function handler(req: NextApiRequest, res: NextApiResponse) { … } export default defaultHandler({ GET: Promise.resolve({ default: defaultResponder(handler) }), }); ``` <!-- review_comment_end --> --- `13-34`: **Add minimal tests: 401 when unauthenticated; URL shape when authenticated.** Covers the main branches and prevents regressions in status handling. - GET /api/integrations/kyzon-space/add without session → 401 + { message: "Unauthorized" } - GET with session → 200 + { url } where url includes: - response_type=code - client_id=<from app keys> - redirect_uri=${WEBAPP_URL}/api/integrations/kyzon-space/callback - scope=meetings:write%20calendar:write%20profile:read - state=<non-empty> I can scaffold these with the project’s preferred test runner if you want. <!-- review_comment_end --> <!-- file_end --> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **💡 Knowledge Base configuration:** - MCP integration is disabled by default for public repositories - Jira integration is disabled by default for public repositories - Linear integration is disabled by default for public repositories You can enable these sources in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between cfc7429879f407c29b5518691b8852451fe99c10 and db81e147e21343dea5743b0a67a65e81f89f1f48. </details> <details> <summary>⛔ Files ignored due to path filters (5)</summary> * `packages/app-store/kyzon-space/static/1.png` is excluded by `!**/*.png` * `packages/app-store/kyzon-space/static/2.png` is excluded by `!**/*.png` * `packages/app-store/kyzon-space/static/3.png` is excluded by `!**/*.png` * `packages/app-store/kyzon-space/static/4.png` is excluded by `!**/*.png` * `packages/app-store/kyzon-space/static/icon.svg` is excluded by `!**/*.svg` </details> <details> <summary>📒 Files selected for processing (21)</summary> * `packages/app-store/apps.keys-schemas.generated.ts` (2 hunks) * `packages/app-store/apps.metadata.generated.ts` (2 hunks) * `packages/app-store/apps.schemas.generated.ts` (2 hunks) * `packages/app-store/apps.server.generated.ts` (1 hunks) * `packages/app-store/bookerApps.metadata.generated.ts` (2 hunks) * `packages/app-store/kyzon-space/DESCRIPTION.md` (1 hunks) * `packages/app-store/kyzon-space/api/add.ts` (1 hunks) * `packages/app-store/kyzon-space/api/callback.ts` (1 hunks) * `packages/app-store/kyzon-space/api/index.ts` (1 hunks) * `packages/app-store/kyzon-space/config.json` (1 hunks) * `packages/app-store/kyzon-space/index.ts` (1 hunks) * `packages/app-store/kyzon-space/lib/KyzonCredentialKey.ts` (1 hunks) * `packages/app-store/kyzon-space/lib/VideoApiAdapter.ts` (1 hunks) * `packages/app-store/kyzon-space/lib/apiTypes.ts` (1 hunks) * `packages/app-store/kyzon-space/lib/axios.ts` (1 hunks) * `packages/app-store/kyzon-space/lib/getKyzonAppKeys.ts` (1 hunks) * `packages/app-store/kyzon-space/lib/index.ts` (1 hunks) * `packages/app-store/kyzon-space/lib/tokenManager.ts` (1 hunks) * `packages/app-store/kyzon-space/package.json` (1 hunks) * `packages/app-store/kyzon-space/zod.ts` (1 hunks) * `packages/app-store/video.adapters.generated.ts` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (18)</summary> * packages/app-store/apps.schemas.generated.ts * packages/app-store/apps.server.generated.ts * packages/app-store/kyzon-space/DESCRIPTION.md * packages/app-store/kyzon-space/zod.ts * packages/app-store/kyzon-space/api/index.ts * packages/app-store/kyzon-space/config.json * packages/app-store/kyzon-space/lib/axios.ts * packages/app-store/bookerApps.metadata.generated.ts * packages/app-store/apps.metadata.generated.ts * packages/app-store/kyzon-space/index.ts * packages/app-store/kyzon-space/lib/index.ts * packages/app-store/apps.keys-schemas.generated.ts * packages/app-store/kyzon-space/package.json * packages/app-store/kyzon-space/lib/getKyzonAppKeys.ts * packages/app-store/kyzon-space/lib/VideoApiAdapter.ts * packages/app-store/kyzon-space/lib/apiTypes.ts * packages/app-store/kyzon-space/api/callback.ts * packages/app-store/kyzon-space/lib/KyzonCredentialKey.ts </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (3)</summary> <details> <summary>**/*.ts</summary> **📄 CodeRabbit inference engine (.cursor/rules/review.mdc)** > `**/*.ts`: For Prisma queries, only select data you need; never use `include`, always use `select` > Ensure the `credential.key` field is never returned from tRPC endpoints or APIs Files: - `packages/app-store/video.adapters.generated.ts` - `packages/app-store/kyzon-space/api/add.ts` - `packages/app-store/kyzon-space/lib/tokenManager.ts` </details> <details> <summary>**/*.{ts,tsx}</summary> **📄 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/video.adapters.generated.ts` - `packages/app-store/kyzon-space/api/add.ts` - `packages/app-store/kyzon-space/lib/tokenManager.ts` </details> <details> <summary>**/*.{ts,tsx,js,jsx}</summary> **⚙️ 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/video.adapters.generated.ts` - `packages/app-store/kyzon-space/api/add.ts` - `packages/app-store/kyzon-space/lib/tokenManager.ts` </details> </details><details> <summary>🧠 Learnings (2)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.156Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.</details> <details> <summary>📚 Learning: 2025-08-08T09:12:08.280Z</summary>Learnt from: hariombalhara
PR: #22968
File: packages/features/auth/lib/next-auth-options.ts:327-327
Timestamp: 2025-08-08T09:12:08.280Z
Learning: In packages/features/auth/lib/next-auth-options.ts, do not log credentials in authorize() handlers (e.g., the "saml-idp" CredentialsProvider). Remove accidental console.log statements and avoid including credential contents in logs; prefer either no logging or structured logs without sensitive data.**Applied to files:** - `packages/app-store/kyzon-space/lib/tokenManager.ts` </details> </details> </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>packages/app-store/video.adapters.generated.ts (2)</summary><blockquote> `12-12`: **LGTM: adapter map entry added correctly and in order.** Dynamic import path and slug look consistent with existing entries. --- `12-12`: **No changes needed: adapter export and slug are consistent.** KyzonSpace’s `VideoApiAdapter` file default-exports `KyzonVideoApiAdapter`, matching other adapters, and the `"kyzon-space"` slug in `config.json` aligns with all generated metadata. </blockquote></details> <details> <summary>packages/app-store/kyzon-space/api/add.ts (1)</summary><blockquote> `26-28`: **Confirm KYZON OAuth scopes and redirect URIs are allowlisted.** Code correctly uses `/api/integrations/kyzon-space/callback` (via `WEBAPP_URL`/`NEXT_PUBLIC_WEBAPP_URL`) and the scope string `"meetings:write calendar:write profile:read"`; verify KYZON’s dashboard includes both `https://cal.com/api/integrations/kyzon-space/callback` and `https://app.cal.com/api/integrations/kyzon-space/callback`, and that the scope list matches their integration spec. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
…e at compile time.
…r redirect_uri to be consistent
… format to guarantee UTC.
…n proper JSON responses
…to avoid runtime surprises
…ount for clock-skew and add runtime validation
…avoid dumping secrets
…fail fast and avoid blocking the app
Fail safe to “expired” if expiry_date is missing or invalid.
URLSearchParams handles RFC 3986 encoding more predictably for OAuth params.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/app-store/kyzon-space/lib/VideoApiAdapter.ts (3)
21-34: Add a one-time 401 refresh-and-retry wrapper.Handles mid-call 401s reliably instead of only proactive refresh.
const getRefreshedKey = async (): Promise<KyzonCredentialKey> => { let key = kyzonCredentialKeySchema.parse(credential.key); @@ return key; }; + + const withAuth = async <T>(op: (key: KyzonCredentialKey) => Promise<T>): Promise<T> => { + let key = await getRefreshedKey(); + try { + return await op(key); + } catch (err: any) { + if (err?.response?.status === 401) { + const refreshed = await refreshKyzonToken(credential.id); + if (refreshed) return await op(refreshed); + } + throw err; + } + };
36-87: Wrap createMeeting with withAuth to auto-recover on 401.Ensures the two-step create flow retries once with a fresh token.
- const createMeeting = async (event: CalendarEvent): Promise<VideoCallData> => { - const key = await getRefreshedKey(); + const createMeeting = async (event: CalendarEvent): Promise<VideoCallData> => { + return withAuth(async (key) => { @@ - return { + return { type: config.type, id: calendarData.id, password: calendarData.meetingPassword || spaceCallData.password, url: calendarData.meetingLink || spaceCallData.url, }; - }; + }); + };
92-141: Apply 401 auto-refresh to updateMeeting as well.Prevents silent fallbacks when a stale token causes 401.
- updateMeeting: async (bookingRef: PartialReference, event: CalendarEvent): Promise<VideoCallData> => { - const key = await getRefreshedKey(); + updateMeeting: async (bookingRef: PartialReference, event: CalendarEvent): Promise<VideoCallData> => { + return withAuth(async (key) => { @@ - return { + return { type: config.type, id: updatedCalendarEvent.id, password: updatedCalendarEvent.meetingPassword || "", url: updatedCalendarEvent.meetingLink || "", }; } catch (error) { // If update fails, return existing meeting data return { type: config.type, id: bookingRef.meetingId, password: bookingRef.meetingPassword || "", url: bookingRef.meetingUrl || "", }; } - }, + }); + },
🧹 Nitpick comments (2)
packages/app-store/kyzon-space/lib/tokenManager.ts (1)
33-41: Treat refresh_token as optional in refresh responses; preserve the old one.Many providers omit refresh_token on refresh. Make the response type optional and fall back to the stored token.
- const { data: newTokens } = await kyzonAxiosInstance.post<{ + const { data: newTokens } = await kyzonAxiosInstance.post<{ access_token: string; - refresh_token: string; + refresh_token?: string; token_type: "Bearer"; expires_in: number; scope: string; }>( @@ - const newCredentialKey = getKyzonCredentialKey({ ...currentKey, ...newTokens }); + const newCredentialKey = getKyzonCredentialKey({ + ...currentKey, + ...newTokens, + refresh_token: newTokens.refresh_token ?? currentKey.refresh_token!, + });Also applies to: 54-55
packages/app-store/kyzon-space/lib/VideoApiAdapter.ts (1)
159-200: Optional: idempotency key for create/update to avoid duplicates.If KYZON supports it, pass an idempotency header using event.uid to de-dupe retries.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
packages/app-store/kyzon-space/api/add.ts(1 hunks)packages/app-store/kyzon-space/api/callback.ts(1 hunks)packages/app-store/kyzon-space/lib/KyzonCredentialKey.ts(1 hunks)packages/app-store/kyzon-space/lib/VideoApiAdapter.ts(1 hunks)packages/app-store/kyzon-space/lib/axios.ts(1 hunks)packages/app-store/kyzon-space/lib/getKyzonAppKeys.ts(1 hunks)packages/app-store/kyzon-space/lib/tokenManager.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/app-store/kyzon-space/lib/axios.ts
- packages/app-store/kyzon-space/lib/getKyzonAppKeys.ts
- packages/app-store/kyzon-space/api/add.ts
🧰 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/kyzon-space/lib/KyzonCredentialKey.tspackages/app-store/kyzon-space/lib/VideoApiAdapter.tspackages/app-store/kyzon-space/api/callback.tspackages/app-store/kyzon-space/lib/tokenManager.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/kyzon-space/lib/KyzonCredentialKey.tspackages/app-store/kyzon-space/lib/VideoApiAdapter.tspackages/app-store/kyzon-space/api/callback.tspackages/app-store/kyzon-space/lib/tokenManager.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/kyzon-space/lib/KyzonCredentialKey.tspackages/app-store/kyzon-space/lib/VideoApiAdapter.tspackages/app-store/kyzon-space/api/callback.tspackages/app-store/kyzon-space/lib/tokenManager.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.156Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
📚 Learning: 2025-08-08T09:12:08.280Z
Learnt from: hariombalhara
PR: calcom/cal.com#22968
File: packages/features/auth/lib/next-auth-options.ts:327-327
Timestamp: 2025-08-08T09:12:08.280Z
Learning: In packages/features/auth/lib/next-auth-options.ts, do not log credentials in authorize() handlers (e.g., the "saml-idp" CredentialsProvider). Remove accidental console.log statements and avoid including credential contents in logs; prefer either no logging or structured logs without sensitive data.
Applied to files:
packages/app-store/kyzon-space/api/callback.tspackages/app-store/kyzon-space/lib/tokenManager.ts
🧬 Code graph analysis (3)
packages/app-store/kyzon-space/lib/VideoApiAdapter.ts (6)
packages/types/VideoApiAdapter.d.ts (1)
VideoApiAdapter(19-49)packages/app-store/kyzon-space/lib/KyzonCredentialKey.ts (2)
KyzonCredentialKey(14-14)kyzonCredentialKeySchema(3-12)packages/app-store/kyzon-space/lib/tokenManager.ts (2)
isTokenExpired(77-86)refreshKyzonToken(11-74)packages/types/Calendar.d.ts (2)
CalendarEvent(163-226)RecurringEvent(129-136)packages/app-store/kyzon-space/lib/axios.ts (1)
kyzonAxiosInstance(5-8)packages/app-store/kyzon-space/lib/apiTypes.ts (7)
KyzonSpaceCallResponse(6-11)KyzonCreateSpaceCallRequestBody(1-4)KyzonGetCalendarEventResponse(46-59)KyzonCreateOrPutCalendarEventRequestBody(20-42)KyzonSingleSpaceCallWithinRangeResponse(66-93)KyzonGetSpaceCallsWithinRangeRequestQuery(61-64)KyzonCalendarEventRecurrence(13-18)
packages/app-store/kyzon-space/api/callback.ts (4)
packages/app-store/kyzon-space/lib/getKyzonAppKeys.ts (1)
getKyzonAppKeys(9-12)packages/app-store/kyzon-space/lib/KyzonCredentialKey.ts (2)
KyzonCredentialKey(14-14)getKyzonCredentialKey(16-34)packages/app-store/kyzon-space/lib/axios.ts (1)
kyzonAxiosInstance(5-8)packages/lib/constants.ts (1)
WEBAPP_URL(12-18)
packages/app-store/kyzon-space/lib/tokenManager.ts (3)
packages/app-store/kyzon-space/lib/KyzonCredentialKey.ts (3)
KyzonCredentialKey(14-14)kyzonCredentialKeySchema(3-12)getKyzonCredentialKey(16-34)packages/app-store/kyzon-space/lib/getKyzonAppKeys.ts (1)
getKyzonAppKeys(9-12)packages/app-store/kyzon-space/lib/axios.ts (1)
kyzonAxiosInstance(5-8)
⏰ 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). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
packages/app-store/kyzon-space/lib/KyzonCredentialKey.ts (1)
3-12: Schema + coercions look solid.Good runtime validation: non-empty tokens, literal "Bearer", integer ms expiry, and string coercion for ids.
packages/app-store/kyzon-space/lib/tokenManager.ts (1)
66-71: Nice: sanitized error logging avoids leaking secrets.Structured logging of status/message/code without dumping Axios config looks good.
packages/app-store/kyzon-space/api/callback.ts (2)
55-79: Verify redirect_uri host matches the one used during authorize.WEBAPP_URL drives redirect_uri; ensure add.ts uses the same host so the provider accepts the code exchange across envs.
109-127: Good: user-facing error without dumping OAuth secrets.Extracting error_description and avoiding raw error logs prevents credential leakage.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.ts (5)
38-53: Handle 429/503 with a small backoff retry.Prevents transient rate-limit/server hiccups from bubbling up.
Apply within this block:
@@ - } catch (error) { - // Only retry on 401 with a different token - if (axios.isAxiosError(error) && error.response?.status === 401) { - const refreshedToken = await refreshKyzonToken(credential.id); - if (refreshedToken && refreshedToken.access_token !== key.access_token) { - return await operation(refreshedToken); - } - } - throw error; - } + } catch (error) { + if (axios.isAxiosError(error)) { + const status = error.response?.status; + // Retry once on 401 with a new token + if (status === 401) { + const refreshedToken = await refreshKyzonToken(credential.id); + if (refreshedToken && refreshedToken.access_token !== key.access_token) { + return await operation(refreshedToken); + } + } + // Soft retry once on rate limiting / transient upstream issues + if (status === 429 || status === 503) { + const retryAfterHeader = error.response?.headers?.["retry-after"]; + const delayMs = Math.min( + (retryAfterHeader ? Number(retryAfterHeader) * 1000 : 1000), + 5000 + ); + await sleep(delayMs); + return await operation(await getRefreshedKey()); + } + } + throw error; + }Add once (outside the above range):
const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
71-101: Sanitize invitee emails and enrich thirdPartySource with a view URL.Avoid sending undefined/duplicate emails and help KYZON link back to Cal.
@@ - invitees: event.attendees?.map((attendee) => ({ - email: attendee.email, - })), + invitees: (event.attendees || []) + .map((a) => a?.email?.trim()) + .filter((email): email is string => Boolean(email)) + .filter((email, i, arr) => arr.indexOf(email) === i) + .map((email) => ({ email })), @@ - thirdPartySource: { + thirdPartySource: { calendarSource: "Cal.com", eventId: event.uid || "", - }, + viewUrl: event.platformBookingUrl ?? event.bookerUrl ?? undefined, + },
168-187: Also delete the underlying Space call to avoid orphans (if not cascaded).Best-effort: fetch the calendar event, then delete the Space call by
meetingId.@@ - deleteMeeting: async (meetingId: string): Promise<void> => { - try { - await authenticatedRequest((key) => - kyzonAxiosInstance.delete(`/v1/teams/${key.team_id}/calendar-events/${meetingId}`, { - headers: { - Authorization: `Bearer ${key.access_token}`, - }, - }) - ); + deleteMeeting: async (meetingId: string): Promise<void> => { + try { + const existing = await authenticatedRequest((key) => + kyzonAxiosInstance.get<KyzonGetCalendarEventResponse>( + `/v1/teams/${key.team_id}/calendar-events/${meetingId}`, + { headers: { Authorization: `Bearer ${key.access_token}` } } + ) + ).then((r) => r.data).catch(() => null); + await authenticatedRequest((key) => + kyzonAxiosInstance.delete(`/v1/teams/${key.team_id}/calendar-events/${meetingId}`, { + headers: { Authorization: `Bearer ${key.access_token}` }, + }) + ); + if (existing?.meetingId) { + await authenticatedRequest((key) => + kyzonAxiosInstance.delete(`/v1/teams/${key.team_id}/space/calls/${existing.meetingId}`, { + headers: { Authorization: `Bearer ${key.access_token}` }, + }) + ).catch(() => void 0); + } } catch (error) {
212-225: Count ongoing/all-day events as busy to prevent double-booking.Fallback end to
dateTowhen API omits it.@@ - return spaceCalls.reduce<EventBusyDate[]>((acc, call) => { - if (!call.eventTime.endTimeUtcISOString) { - // ongoing / all-day event, don't count it as a busy date - return acc; - } - - acc.push({ - start: call.eventTime.startTimeUtcISOString, - end: call.eventTime.endTimeUtcISOString, - source: "KYZON Space", - }); + return spaceCalls.reduce<EventBusyDate[]>((acc, call) => { + const endIso = call.eventTime.endTimeUtcISOString ?? dateTo; + if (!endIso) return acc; + acc.push({ + start: call.eventTime.startTimeUtcISOString, + end: endIso, + source: "KYZON Space", + }); return acc; }, []);
239-239: Prefer named export per repo guideline (avoid default exports).-export default KyzonVideoApiAdapter; +export { KyzonVideoApiAdapter };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.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/kyzonspacevideo/lib/VideoApiAdapter.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/kyzonspacevideo/lib/VideoApiAdapter.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/kyzonspacevideo/lib/VideoApiAdapter.ts
🧠 Learnings (3)
📓 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.942Z
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.942Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/config.json:5-5
Timestamp: 2025-09-01T07:31:00.942Z
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/kyzonspacevideo/lib/VideoApiAdapter.ts
📚 Learning: 2025-09-01T07:31:00.942Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/config.json:5-5
Timestamp: 2025-09-01T07:31:00.942Z
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/kyzonspacevideo/lib/VideoApiAdapter.ts
🧬 Code graph analysis (1)
packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.ts (6)
packages/types/VideoApiAdapter.d.ts (1)
VideoApiAdapter(19-49)packages/app-store/kyzonspacevideo/lib/KyzonCredentialKey.ts (2)
KyzonCredentialKey(14-14)kyzonCredentialKeySchema(3-12)packages/app-store/kyzonspacevideo/lib/tokenManager.ts (2)
isTokenExpired(90-99)refreshKyzonToken(13-21)packages/types/Calendar.d.ts (2)
CalendarEvent(163-226)RecurringEvent(129-136)packages/app-store/kyzonspacevideo/lib/axios.ts (1)
kyzonAxiosInstance(5-9)packages/app-store/kyzonspacevideo/lib/apiTypes.ts (7)
KyzonSpaceCallResponse(6-11)KyzonCreateSpaceCallRequestBody(1-4)KyzonGetCalendarEventResponse(46-59)KyzonCreateOrPutCalendarEventRequestBody(20-42)KyzonSingleSpaceCallWithinRangeResponse(66-93)KyzonGetSpaceCallsWithinRangeRequestQuery(61-64)KyzonCalendarEventRecurrence(13-18)
⏰ 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). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (3)
packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.ts (3)
24-36: Token refresh path looks solid.Schema-validated parse + single-flight refresh usage is clean.
244-279: Recurrence conversion looks correct.Mappings and guards align with supported frequencies.
103-108: No change required:config.typecorrectly uses underscores and is normalized at runtime
VideoClient.ts splitsconfig.typeon_(e.g."kyzonspace_video" → "kyzonspacevideo"), matching the generated adapter key.Likely an incorrect or invalid review comment.
Devanshusharma2005
left a comment
Devanshusharma2005
left a comment
There was a problem hiding this comment.
lets add tests also to verify this.
| - 4.png | ||
| --- | ||
|
|
||
| {DESCRIPTION} |
There was a problem hiding this comment.
we don't need to add description . You can remove this.
There was a problem hiding this comment.
When I removed {DESCRIPTION}, my app's description disappears from the app store page, so I think we do still need it
|
making the OAuth callback URLs configurable instead of hardcoded. It’d also be great to improve error recovery with more user-friendly messages . Overall, solid pr just needs a bit more polish! |
|
Thank you for getting back to us quickly! Just to clarify on this:
Did you mean making the OAuth callback URL in this PR configurable, or making the whitelisted callback URLs on the KYZON server configurable? If you had meant the urls that are whitelisted on the KYZON server, because of security concerns, we won't be able to allow user-configurable redirect URIs / open the whitelist to arbitrary/self-hosted domains. If your request was about Cal.com needing additional callback URLs (e.g. staging domains), just let us know and we'd be happy to update our configuration. 😊 E.g. we should be able to expand to cover all subdomains under The furthest we would probably be able to safely go would be: if you're anticipating that the path to the integration might change in the future. Thisll cover all paths on Cal-controlled domains (but we'd prefer to keep it scoped to the integration path unless there’s a clear need!) |
also handle when updateMeeting 404s, now we will create the meeting if it doesnt exist when updating it
|
@Devanshusharma2005 I think I've covered all the points you've raised so far in your comments. Thank you for the PR review! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (17)
packages/app-store/kyzonspacevideo/lib/tokenManager.test.ts (3)
52-57: Stabilize time-dependent tests to avoid flakiness.Freeze the clock so Date.now()-based expectations can’t race on slow CI.
Apply:
describe("tokenManager", () => { - beforeEach(() => { - vi.clearAllMocks(); - mockGetKyzonAppKeys.mockResolvedValue(mockAppKeys); - }); + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2024-01-15T10:00:00Z")); + vi.clearAllMocks(); + mockGetKyzonAppKeys.mockResolvedValue(mockAppKeys); + }); + afterEach(() => { + vi.useRealTimers(); + });Also applies to: 58-76
302-364: Add a post-resolution call to verify in-flight de-dup map is cleared.After concurrent resolution, call refresh again and expect the DB/API/update path to be invoked a second time.
Append:
test("prevents concurrent refresh requests for same credential", async () => { @@ expect(prismaMock.credential.update).toHaveBeenCalledTimes(1); }); + + test("allows a new refresh after previous one completes", async () => { + const credentialId = 123; + prismaMock.credential.findUnique.mockResolvedValueOnce({ id: credentialId, key: mockCredentialKey }); + mockPost.mockResolvedValueOnce({ data: { access_token: "a2", refresh_token: "r2", token_type: "Bearer", expires_in: 3600, scope: mockCredentialKey.scope } }); + mockGetKyzonCredentialKey.mockReturnValueOnce({ ...mockCredentialKey, access_token: "a2" }); + prismaMock.credential.update.mockResolvedValueOnce({ id: credentialId, key: { ...mockCredentialKey, access_token: "a2" } as any }); + + await refreshKyzonToken(credentialId); + + prismaMock.credential.findUnique.mockResolvedValueOnce({ id: credentialId, key: { ...mockCredentialKey, access_token: "a2" } }); + mockPost.mockResolvedValueOnce({ data: { access_token: "a3", refresh_token: "r3", token_type: "Bearer", expires_in: 3600, scope: mockCredentialKey.scope } }); + mockGetKyzonCredentialKey.mockReturnValueOnce({ ...mockCredentialKey, access_token: "a3" }); + prismaMock.credential.update.mockResolvedValueOnce({ id: credentialId, key: { ...mockCredentialKey, access_token: "a3" } as any }); + + await refreshKyzonToken(credentialId); + + expect(prismaMock.credential.findUnique).toHaveBeenCalledTimes(2); + expect(mockPost).toHaveBeenCalledTimes(2); + expect(prismaMock.credential.update).toHaveBeenCalledTimes(2); + });
236-261: Differentiate log messages by scenario for easier SRE triage.Use consistent prefixes and include a short “action” hint (e.g., “reconnect account”) in logs.
Apply:
- console.error("Failed to refresh KYZON token", + console.error("[kyzonspace] token refresh failed", expect.objectContaining({ message: "Refresh token expired", }) );packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.ts (3)
55-110: Consider idempotency and partial-creation cleanup on createMeeting.If calendar-event creation fails after the space call is created, you may leave an orphaned call. If KYZON supports it, pass an Idempotency-Key (e.g., event.uid) and/or best-effort cleanup the created call on the error path.
Example (guarded, only if API supports):
- const { data: spaceCallData } = await authenticatedRequest((key) => + const { data: spaceCallData } = await authenticatedRequest((key) => kyzonAxiosInstance.post<KyzonSpaceCallResponse>( `/v1/teams/${key.team_id}/space/calls`, { name: event.title, isScheduled: true, } satisfies KyzonCreateSpaceCallRequestBody, { headers: { Authorization: `Bearer ${key.access_token}`, + // "Idempotency-Key": event.uid ?? undefined, }, } ) ); @@ - } catch (error) { + } catch (error) { const err = error as any; + // Optional: if calendar-event creation failed after creating a space call, attempt cleanup here. + // Swallow cleanup errors.
236-282: Availability parsing is fine; minor guard optional.You could assert the union shape before reading endTime to future-proof against API drift.
Example:
- if (!call.eventTime.endTimeUtcISOString) { + if (!("endTimeUtcISOString" in call.eventTime) || !call.eventTime.endTimeUtcISOString) { return acc; }
286-286: Prefer named export over default export.Aligns with repo guidance for better tree-shaking and refactors.
Apply:
-export default KyzonVideoApiAdapter; +export { KyzonVideoApiAdapter };And update imports at call sites/tests accordingly.
packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.test.ts (3)
232-266: Remove unnecessary axios doMock; rely on isAxiosError property.The adapter already uses axios.isAxiosError which returns true when error.isAxiosError === true; late doMock likely won’t affect the already-imported module.
Apply:
- vi.doMock("axios", () => ({ - isAxiosError: vi.fn(() => true), - }));
268-305: Add test for update 404 → create fallback path.Covers the documented behavior and ensures we don’t regress.
Append:
+ test("update 404 triggers createMeeting fallback", async () => { + const bookingRef = { meetingId: "missing" }; + const notFound = Object.assign(new Error("Not found"), { isAxiosError: true, response: { status: 404 } }); + mockPut.mockRejectedValueOnce(notFound); + mockPost + .mockResolvedValueOnce({ data: { id: "space_call_x", url: "u", password: "p" } }) + .mockResolvedValueOnce({ data: { id: "calendar_event_x", meetingLink: "U", meetingPassword: "P" } }); + + const api = KyzonVideoApiAdapter(testCredential)!; + const res = await api.updateMeeting(bookingRef, mockCalendarEvent); + expect(res.id).toBe("calendar_event_x"); + });
199-231: Assert friendly error mapping for common statuses (403, 429).Add expectations for the user-facing messages to lock in UX.
Example (403):
+ test("createMeeting maps 403 to permission message", async () => { + const err = { isAxiosError: true, response: { status: 403 } }; + mockPost.mockRejectedValueOnce(err); + const api = KyzonVideoApiAdapter(testCredential)!; + await expect(api.createMeeting(mockCalendarEvent)).rejects.toThrow( + "You don't have permission to create meetings in KYZON Space. Please check your account permissions." + ); + });packages/app-store/kyzonspacevideo/api/callback.ts (5)
119-121: Verify redirect_uri host strictly matches KYZON’s whitelist.WEBAPP_URL can vary across envs (and may lack scheme). If KYZON only whitelists app.cal.com and cal.com, ensure this always resolves to a whitelisted origin in Cloud; otherwise installs will fail on staging/self-hosted. Consider a canonical base (env like CAL_PUBLIC_BASE_URL_FOR_OAUTH) gated to Cloud.
17-22: Guard HTTP method.Lock the handler to GET and return 405 for anything else.
export default async function handler(req: NextApiRequest, res: NextApiResponse) { + if (req.method !== "GET") { + res.setHeader("Allow", "GET"); + return res.status(405).end("Method Not Allowed"); + } const state = decodeOAuthState(req); const userId = req.session?.user.id;
162-168: Avoid logging full response.data; may contain PII/secrets.Log minimal structured fields.
- console.error("KYZON OAuth callback error:", { - status: axiosError?.response?.status, - message: axiosError?.message, - code: axiosError?.code, - data: axiosError?.response?.data, - hasState: !!(state?.onErrorReturnTo || state?.returnTo), - }); + console.error("KYZON OAuth callback error:", { + status: axiosError?.response?.status, + message: axiosError?.message, + code: axiosError?.code, + error: axiosError?.response?.data?.error, + hasState: !!(state?.onErrorReturnTo || state?.returnTo), + });
145-148: Minor: Use token_type for header construction.Future-proof if KYZON ever changes type (still validates with “Bearer” today).
- Authorization: `Bearer ${authorizeResult.access_token}`, + Authorization: `${authorizeResult.token_type} ${authorizeResult.access_token}`,
235-246: Prevent duplicate credentials under concurrency.Delete-then-create can race. Prefer a unique composite index (userId, appId, type) and an upsert, or wrap delete+create in a single transaction.
Would you like a migration + upsert sketch?
packages/app-store/kyzonspacevideo/api/callback.test.ts (3)
289-301: Add fallback redirect test when all safe URLs are null.Assert it falls back to getInstalledAppPath ("/apps/kyzonspacevideo").
- Mock getSafeRedirectUrl to always return null and expect res.redirect("/apps/kyzonspacevideo").
429-451: Exercise 429/503 mappings and ECONNREFUSED/ENOTFOUND branches.Add tests for rate limiting and maintenance messages, and for Axios errors with code set to ECONNREFUSED/ENOTFOUND.
69-80: Add a 405 test if method guard is applied.Create a request with method "POST" and expect status 405 and Allow: GET.
Also applies to: 81-90
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
packages/app-store/kyzonspacevideo/api/callback.test.ts(1 hunks)packages/app-store/kyzonspacevideo/api/callback.ts(1 hunks)packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.test.ts(1 hunks)packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.ts(1 hunks)packages/app-store/kyzonspacevideo/lib/tokenManager.test.ts(1 hunks)packages/app-store/kyzonspacevideo/lib/tokenManager.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/app-store/kyzonspacevideo/lib/tokenManager.ts
🧰 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/kyzonspacevideo/lib/tokenManager.test.tspackages/app-store/kyzonspacevideo/lib/VideoApiAdapter.tspackages/app-store/kyzonspacevideo/lib/VideoApiAdapter.test.tspackages/app-store/kyzonspacevideo/api/callback.tspackages/app-store/kyzonspacevideo/api/callback.test.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/kyzonspacevideo/lib/tokenManager.test.tspackages/app-store/kyzonspacevideo/lib/VideoApiAdapter.tspackages/app-store/kyzonspacevideo/lib/VideoApiAdapter.test.tspackages/app-store/kyzonspacevideo/api/callback.tspackages/app-store/kyzonspacevideo/api/callback.test.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/kyzonspacevideo/lib/tokenManager.test.tspackages/app-store/kyzonspacevideo/lib/VideoApiAdapter.tspackages/app-store/kyzonspacevideo/lib/VideoApiAdapter.test.tspackages/app-store/kyzonspacevideo/api/callback.tspackages/app-store/kyzonspacevideo/api/callback.test.ts
🧠 Learnings (5)
📓 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.942Z
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-01T08:56:14.049Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/lib/tokenManager.ts:25-31
Timestamp: 2025-09-01T08:56:14.049Z
Learning: In token refresh utilities like tokenManager.ts, accessing credential.key from Prisma is legitimate and necessary for OAuth token refresh flows. These internal utilities need stored credentials to refresh tokens and don't expose them in API responses.
Applied to files:
packages/app-store/kyzonspacevideo/lib/tokenManager.test.ts
📚 Learning: 2025-09-01T07:31:00.942Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/config.json:5-5
Timestamp: 2025-09-01T07:31:00.942Z
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/kyzonspacevideo/lib/VideoApiAdapter.ts
📚 Learning: 2025-09-01T07:31:00.942Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/config.json:5-5
Timestamp: 2025-09-01T07:31:00.942Z
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/kyzonspacevideo/lib/VideoApiAdapter.ts
📚 Learning: 2025-09-01T10:57:36.963Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.ts:119-149
Timestamp: 2025-09-01T10:57:36.963Z
Learning: In the KYZON API, when performing PUT operations on calendar events, omitting the location field will retain the existing location.spaceCallId rather than clearing it. This means the location field behaves more like a PATCH operation - only updating fields that are explicitly provided.
Applied to files:
packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.ts
🧬 Code graph analysis (5)
packages/app-store/kyzonspacevideo/lib/tokenManager.test.ts (4)
packages/app-store/kyzonspacevideo/lib/axios.ts (1)
kyzonAxiosInstance(5-9)packages/app-store/kyzonspacevideo/lib/getKyzonAppKeys.ts (1)
getKyzonAppKeys(9-12)packages/app-store/kyzonspacevideo/lib/KyzonCredentialKey.ts (2)
getKyzonCredentialKey(16-34)KyzonCredentialKey(14-14)packages/app-store/kyzonspacevideo/lib/tokenManager.ts (2)
isTokenExpired(122-131)refreshKyzonToken(13-21)
packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.ts (6)
packages/types/VideoApiAdapter.d.ts (1)
VideoApiAdapter(19-49)packages/app-store/kyzonspacevideo/lib/KyzonCredentialKey.ts (2)
KyzonCredentialKey(14-14)kyzonCredentialKeySchema(3-12)packages/app-store/kyzonspacevideo/lib/tokenManager.ts (2)
isTokenExpired(122-131)refreshKyzonToken(13-21)packages/types/Calendar.d.ts (2)
CalendarEvent(163-226)RecurringEvent(129-136)packages/app-store/kyzonspacevideo/lib/axios.ts (1)
kyzonAxiosInstance(5-9)packages/app-store/kyzonspacevideo/lib/apiTypes.ts (7)
KyzonSpaceCallResponse(6-11)KyzonCreateSpaceCallRequestBody(1-4)KyzonGetCalendarEventResponse(46-59)KyzonCreateOrPutCalendarEventRequestBody(20-42)KyzonSingleSpaceCallWithinRangeResponse(66-93)KyzonGetSpaceCallsWithinRangeRequestQuery(61-64)KyzonCalendarEventRecurrence(13-18)
packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.test.ts (4)
packages/app-store/kyzonspacevideo/lib/axios.ts (1)
kyzonAxiosInstance(5-9)packages/app-store/kyzonspacevideo/lib/tokenManager.ts (2)
refreshKyzonToken(13-21)isTokenExpired(122-131)packages/app-store/kyzonspacevideo/lib/KyzonCredentialKey.ts (1)
KyzonCredentialKey(14-14)packages/types/Calendar.d.ts (1)
CalendarEvent(163-226)
packages/app-store/kyzonspacevideo/api/callback.ts (4)
packages/app-store/kyzonspacevideo/lib/getKyzonAppKeys.ts (1)
getKyzonAppKeys(9-12)packages/app-store/kyzonspacevideo/lib/KyzonCredentialKey.ts (2)
KyzonCredentialKey(14-14)getKyzonCredentialKey(16-34)packages/app-store/kyzonspacevideo/lib/axios.ts (1)
kyzonAxiosInstance(5-9)packages/lib/constants.ts (1)
WEBAPP_URL(12-18)
packages/app-store/kyzonspacevideo/api/callback.test.ts (1)
packages/app-store/kyzonspacevideo/lib/axios.ts (1)
kyzonAxiosInstance(5-9)
🔇 Additional comments (9)
packages/app-store/kyzonspacevideo/lib/tokenManager.test.ts (1)
58-103: Good coverage for token-expiry edge cases.Covers expired, valid, missing, invalid type, and NaN. Solid.
packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.ts (5)
23-36: Runtime-validated credential parsing and preflight refresh look good.Schema-parse + conditional refresh is straightforward and safe.
38-53: 401 retry logic is correct and loop-safe.Narrow retry to 401 and only if token changed—nice.
120-138: User-facing error messages are clear and actionable.Good mapping of 403/429/5xx/network to friendly guidance.
149-172: Omitting location on PUT is correct per KYZON semantics.Given PUT retains existing location when omitted, this is the right behavior.
291-326: Recurrence conversion looks correct and guards unsupported freqs.Mapping and clamping are sensible.
packages/app-store/kyzonspacevideo/lib/VideoApiAdapter.test.ts (1)
96-146: Create flow tests read well and validate both requests and return shape.Nice coverage of the two-step creation.
packages/app-store/kyzonspacevideo/api/callback.ts (1)
32-39: Safe redirect flow looks solid.Prioritizing onErrorReturnTo → returnTo → installed app path with getSafeRedirectUrl is correct.
Also applies to: 83-91, 170-176, 251-253
packages/app-store/kyzonspacevideo/api/callback.test.ts (1)
102-162: End-to-end success path coverage is strong.Mocks, assertions for token/profile calls, credential deletion, creation, and redirect are appropriate.
|
Hi @Devanshusharma2005 @CarinaWolli @anikdhabal, any updates on this? |
|
This PR is being marked as stale due to inactivity. |
|
@romitg2 I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 30 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/app-store/kyzonspacevideo/lib/KyzonCredentialKey.ts">
<violation number="1" location="packages/app-store/kyzonspacevideo/lib/KyzonCredentialKey.ts:6">
P2: token_type validation is case-sensitive; compliant OAuth providers may return lowercase or mixed-case values, causing schema parsing to fail.</violation>
<violation number="2" location="packages/app-store/kyzonspacevideo/lib/KyzonCredentialKey.ts:10">
P2: z.coerce.string() will coerce undefined/null to "undefined"/"null", so required user_id/team_id can pass validation and be persisted as literal strings. This risks corrupting credential data when upstream payloads omit these fields.</violation>
</file>
<file name="packages/app-store/kyzonspacevideo/api/callback.ts">
<violation number="1" location="packages/app-store/kyzonspacevideo/api/callback.ts:79">
P1: Rule violated: **Avoid Logging Sensitive Information**
The OAuth authorization `code` is a sensitive credential (exchangeable for access tokens) and should not be logged. Remove `codeValue: code` from the console.error output—logging the type (`codeType`) is sufficient for debugging.</violation>
<violation number="2" location="packages/app-store/kyzonspacevideo/api/callback.ts:166">
P1: Rule violated: **Avoid Logging Sensitive Information**
Logging `data: axiosError?.response?.data` could expose sensitive OAuth token endpoint response data (the response type includes `access_token` and `refresh_token` fields). Log only the error code and description instead of the full response body.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| status: axiosError?.response?.status, | ||
| message: axiosError?.message, | ||
| code: axiosError?.code, | ||
| data: axiosError?.response?.data, |
There was a problem hiding this comment.
P1: Rule violated: Avoid Logging Sensitive Information
Logging data: axiosError?.response?.data could expose sensitive OAuth token endpoint response data (the response type includes access_token and refresh_token fields). Log only the error code and description instead of the full response body.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/kyzonspacevideo/api/callback.ts, line 166:
<comment>Logging `data: axiosError?.response?.data` could expose sensitive OAuth token endpoint response data (the response type includes `access_token` and `refresh_token` fields). Log only the error code and description instead of the full response body.</comment>
<file context>
@@ -0,0 +1,254 @@
+ status: axiosError?.response?.status,
+ message: axiosError?.message,
+ code: axiosError?.code,
+ data: axiosError?.response?.data,
+ hasState: !!(state?.onErrorReturnTo || state?.returnTo),
+ });
</file context>
| if (typeof code !== "string") { | ||
| console.error("KYZON OAuth callback missing authorization code:", { | ||
| codeType: typeof code, | ||
| codeValue: code, |
There was a problem hiding this comment.
P1: Rule violated: Avoid Logging Sensitive Information
The OAuth authorization code is a sensitive credential (exchangeable for access tokens) and should not be logged. Remove codeValue: code from the console.error output—logging the type (codeType) is sufficient for debugging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/kyzonspacevideo/api/callback.ts, line 79:
<comment>The OAuth authorization `code` is a sensitive credential (exchangeable for access tokens) and should not be logged. Remove `codeValue: code` from the console.error output—logging the type (`codeType`) is sufficient for debugging.</comment>
<file context>
@@ -0,0 +1,254 @@
+ if (typeof code !== "string") {
+ console.error("KYZON OAuth callback missing authorization code:", {
+ codeType: typeof code,
+ codeValue: code,
+ hasState: !!(state?.onErrorReturnTo || state?.returnTo),
+ });
</file context>
| export const kyzonCredentialKeySchema = z.object({ | ||
| access_token: z.string().min(1), | ||
| refresh_token: z.string().min(1).optional(), | ||
| token_type: z.literal("Bearer").optional(), |
There was a problem hiding this comment.
P2: token_type validation is case-sensitive; compliant OAuth providers may return lowercase or mixed-case values, causing schema parsing to fail.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/kyzonspacevideo/lib/KyzonCredentialKey.ts, line 6:
<comment>token_type validation is case-sensitive; compliant OAuth providers may return lowercase or mixed-case values, causing schema parsing to fail.</comment>
<file context>
@@ -0,0 +1,34 @@
+export const kyzonCredentialKeySchema = z.object({
+ access_token: z.string().min(1),
+ refresh_token: z.string().min(1).optional(),
+ token_type: z.literal("Bearer").optional(),
+ scope: z.string().optional(),
+ /* store ms since epoch, > now */
</file context>
| scope: z.string().optional(), | ||
| /* store ms since epoch, > now */ | ||
| expiry_date: z.number().int().positive(), | ||
| user_id: z.coerce.string().min(1), |
There was a problem hiding this comment.
P2: z.coerce.string() will coerce undefined/null to "undefined"/"null", so required user_id/team_id can pass validation and be persisted as literal strings. This risks corrupting credential data when upstream payloads omit these fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/kyzonspacevideo/lib/KyzonCredentialKey.ts, line 10:
<comment>z.coerce.string() will coerce undefined/null to "undefined"/"null", so required user_id/team_id can pass validation and be persisted as literal strings. This risks corrupting credential data when upstream payloads omit these fields.</comment>
<file context>
@@ -0,0 +1,34 @@
+ scope: z.string().optional(),
+ /* store ms since epoch, > now */
+ expiry_date: z.number().int().positive(),
+ user_id: z.coerce.string().min(1),
+ team_id: z.coerce.string().min(1),
+});
</file context>
|
Hey @nangelina sorry for keeping you wait, could you please check comments by cubic. Thanks |
4 participants
What does this PR do?
Your friends at KYZON Solutions are excited to introduce the KYZON Space conferencing app for Cal.com.
When installed, users can set KYZON Space as the location for their event types. Each booking will automatically generate a unique Space call link, ensuring a fresh room for every meeting.
The app integrates with the KYZON API via OAuth, storing an access token for each user. This token is then used to request and attach new Space call links to bookings.
To build this integration, I followed the How to Build an App guide and drew on patterns from existing video conferencing apps (e.g. Zoom, Teams, Jelly) to implement dynamic link generation.
Although the contributor docs generally recommend not committing lock files, based on this previous PR, I included it here since we’re introducing a new package to the monorepo.
Visual Demo
Video Demo:
Screen.Recording.2025-09-01.at.3.40.57.PM.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Please contact me at angelina@kyzonsolutions.com for the
client_id,client_secret, andapi_keythat you'll need to set to test this change.Currently the only OAuth callback URLs we've been configured to accept are:
https://app.cal.com/api/integrations/kyzonspacevideo/callback, andhttps://cal.com/api/integrations/kyzonspacevideo/callback.If you plan to test this with different OAuth callback URL, please let me know so that I can configure our server to accept those requests!