feat: async tasker for google calendar events#23494
feat: async tasker for google calendar events#23494ThyMinimalDev wants to merge 7 commits intomainfrom
Conversation
WalkthroughAdds a tasker framework and Google Calendar taskers across the monorepo: new ICalendarTasker type, a generic BaseTaskerService for safe dispatch with fallback, GoogleCalendarTriggerDevTasker and GoogleCalendarSyncTasker implementations, and a GoogleCalendarTasker aggregator. Introduces a new @calcom/trigger package (tasks, config, re-exports). Exposes taskers from packages/platform/libraries (package version -> 9.9.9 and new ./taskers export) and updates Vite config and TS paths. Integrates taskers into apps/api/v2 (Nest services and GoogleCalendarTaskerModule), adds LoggerAdapter and DI modules/tokens/containers, and adds a workspace dependency to apps/web. 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
|
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
keithwillcode
added
core
| import { Logger } from "@/lib/logger.bridge"; | ||
| import { GoogleCalendarSyncTasker } from "@/lib/services/taskers/google-calendar-sync-tasker.service"; | ||
| import { GoogleCalendarTasker } from "@/lib/services/taskers/google-calendar-tasker.service"; | ||
| import { GoogleCalendarTriggerDevTasker } from "@/lib/services/taskers/google-calendar-trigger-tasker.service"; | ||
| import { Module } from "@nestjs/common"; | ||
|
|
||
| @Module({ | ||
| imports: [], | ||
| providers: [GoogleCalendarSyncTasker, GoogleCalendarTriggerDevTasker, Logger, GoogleCalendarTasker], | ||
| exports: [GoogleCalendarTasker], | ||
| }) | ||
| export class GoogleCalendarTaskerModule {} |
There was a problem hiding this comment.
module which can be used within api v2 controller to inject google calendar tasker
dosubot
bot
added
calendar-apps
| import { nanoid } from "nanoid"; | ||
|
|
||
| import type { ITaskerDependencies } from "../types/tasker"; | ||
| import type { ICalendarTasker } from "./calendar"; | ||
|
|
||
| export class GoogleCalendarSyncTasker implements ICalendarTasker { | ||
| constructor(public readonly dependencies: ITaskerDependencies) {} | ||
|
|
||
| async createEvent(payload: Parameters<ICalendarTasker["createEvent"]>[0]) { | ||
| this.dependencies.logger.info(`Hello ${payload.name}`); | ||
| const runId = `sync_${nanoid(10)}`; | ||
| return { runId }; | ||
| } | ||
| async updateEvent(payload: Parameters<ICalendarTasker["updateEvent"]>[0]) { | ||
| this.dependencies.logger.info(`Hello ${payload.surname}`); | ||
| const runId = `sync_${nanoid(10)}`; | ||
| return { runId }; | ||
| } | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| export class GoogleCalendarTasker extends BaseTaskerService<ICalendarTasker> { | ||
| constructor(public readonly dependencies: ICalendarTaskerDependencies) { | ||
| super(dependencies); | ||
| } | ||
| } |
There was a problem hiding this comment.
main GoogleCalendarTasker interface, typed using BaseTaskerService to make sure typescript is able to typecheck the dispatch as shown in the comments right below
There was a problem hiding this comment.
this is the class in which we will inject the primary / fallback tasker
| import { create } from "@calcom/trigger/tasks/calendars/create"; | ||
|
|
||
| import type { ITaskerDependencies } from "../types/tasker"; | ||
| import type { ICalendarTasker } from "./calendar"; | ||
|
|
||
| export class GoogleCalendarTriggerDevTasker implements ICalendarTasker { | ||
| constructor(public readonly dependencies: ITaskerDependencies) {} | ||
|
|
||
| async createEvent(payload: Parameters<ICalendarTasker["createEvent"]>[0]) { | ||
| this.dependencies.logger.debug("createEvent", payload); | ||
| const handle = await create.trigger({ name: payload.name }); | ||
| this.dependencies.logger.debug("createEvent handle id", handle.id); | ||
| return { runId: handle.id }; | ||
| } | ||
|
|
||
| async updateEvent(payload: Parameters<ICalendarTasker["updateEvent"]>[0]) { | ||
| this.dependencies.logger.debug("createEvent", payload); | ||
| const handle = await create.trigger({ name: payload.surname }); | ||
| this.dependencies.logger.debug("createEvent handle id", handle.id); | ||
| return { runId: handle.id }; | ||
| } | ||
| } |
There was a problem hiding this comment.
skeleton for the trigger.dev tasker, in this file we will import the google calendar tasks defined in packages/trigger
| // 1. The interface for the logger's configuration settings. | ||
| // This is extracted directly from your class. | ||
| export interface IMyLoggerSettings { | ||
| minLevel: number; | ||
| displayTimestamp: boolean; | ||
| logFormat: "pretty" | "json" | "simple"; | ||
| } | ||
|
|
||
| // 2. The main interface for the Logger instance. | ||
| // This describes the public "shape" of your Logger class. | ||
| export interface ILogger { | ||
| /** | ||
| * The configuration settings for this logger instance. | ||
| */ | ||
| readonly settings: IMyLoggerSettings; | ||
|
|
||
| /** | ||
| * Creates a new Logger instance with a specific prefix for contextual logging. | ||
| * @param options - Configuration for the sub-logger. | ||
| * @returns A new ILogger instance with the specified prefix. | ||
| */ | ||
| getSubLogger(options: { prefix?: string[] }): ILogger; | ||
|
|
||
| /** Logs an informational message. */ | ||
| info(...args: any[]): void; | ||
|
|
||
| /** Logs a warning message. */ | ||
| warn(...args: any[]): void; | ||
|
|
||
| /** Logs an error message. */ | ||
| error(...args: any[]): void; | ||
|
|
||
| /** Logs a debug message. */ | ||
| debug(...args: any[]): void; | ||
|
|
||
| /** Logs a detailed trace message. */ | ||
| trace(...args: any[]): void; | ||
|
|
||
| /** Logs a critical error that may require immediate attention. */ | ||
| fatal(...args: any[]): void; | ||
|
|
||
| /** Logs a message with the lowest level of detail. */ | ||
| silly(...args: any[]): void; | ||
| } |
There was a problem hiding this comment.
interface of a logger than can be dependency injected, so we can DI trigger.dev logger, tslog, or api v2 logger
| // No need for a generic base 'TaskerInterface'. We will use the specific domain interface directly. | ||
| // For example, ICalendarTasker. | ||
| import type { ILogger } from "./types/logger"; | ||
|
|
||
| export abstract class BaseTaskerService<T> { | ||
| // T is now just a placeholder for the class that extends this | ||
| // The dependencies are now typed with the specific interfaces | ||
| protected readonly primaryTasker: T; | ||
| protected readonly fallbackTasker: T; | ||
| protected readonly logger: ILogger; | ||
|
|
||
| constructor(dependencies: { primaryTasker: T; fallbackTasker: T; logger: ILogger }) { | ||
| this.primaryTasker = dependencies.primaryTasker; | ||
| this.fallbackTasker = dependencies.fallbackTasker; | ||
| this.logger = dependencies.logger; | ||
| } | ||
|
|
||
| public async dispatch<K extends keyof T>( | ||
| taskName: K, | ||
| ...args: T[K] extends (...args: any[]) => any ? Parameters<T[K]> : never | ||
| ): Promise<T[K] extends (...args: any[]) => any ? Awaited<ReturnType<T[K]>> : never> { | ||
| this.logger.info(`Dispatching task '${String(taskName)}'`, { args }); | ||
| return this.safeDispatch(taskName, ...args); | ||
| } | ||
|
|
||
| // The dispatch method is now strongly typed to the keys of T | ||
| protected async safeDispatch<K extends keyof T>( | ||
| taskName: K, | ||
| // We infer the arguments directly from the method on T | ||
| ...args: T[K] extends (...args: any[]) => any ? Parameters<T[K]> : never | ||
| ): Promise<T[K] extends (...args: any[]) => any ? Awaited<ReturnType<T[K]>> : never> { | ||
| try { | ||
| const method = this.primaryTasker[taskName] as (...args: any[]) => any; | ||
| return await method.apply(this.primaryTasker, args); | ||
| } catch (err) { | ||
| this.logger.error(`Primary tasker failed for '${String(taskName)}'.`, err as Error); | ||
|
|
||
| if (this.primaryTasker === this.fallbackTasker) { | ||
| throw err; | ||
| } | ||
|
|
||
| this.logger.warn(`Falling back to SyncTasker for '${String(taskName)}'.`); | ||
|
|
||
| const fallbackMethod = this.fallbackTasker[taskName] as (...args: any[]) => any; | ||
| return fallbackMethod.apply(this.fallbackTasker, args); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
this is the base tasker class which specialized taskers will extends, this ensures the fallback tasker is called if the primary tasker fails (for example is trigger.dev is down)
There was a problem hiding this comment.
this also abstract the logic of dispatch and safeDispatch so that people implementing Specialized taskers do not have to worry with it
| export { GoogleCalendarTasker } from "@calcom/features/tasker/calendars/GoogleCalendarTasker"; | ||
| export { GoogleCalendarSyncTasker } from "@calcom/features/tasker/calendars/GoogleCalendarSyncTasker"; | ||
| export { GoogleCalendarTriggerDevTasker } from "@calcom/features/tasker/calendars/GoogleCalendarTriggerTasker"; |
There was a problem hiding this comment.
exporting tassker so that apiV2 can build it's own modules with them
| import { schemaTask, logger } from "@trigger.dev/sdk"; | ||
| import { z } from "zod"; | ||
|
|
||
| export const create = schemaTask({ | ||
| id: "calendar.google.create.event", | ||
| schema: z.object({ | ||
| name: z.string(), | ||
| }), | ||
| run: async (payload: { name: string }) => { | ||
| logger.info(`Hello create ${payload.name}!`); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
example trigger.dev task
There was a problem hiding this comment.
we should aim for something like so
import { schemaTask, logger } from "@trigger.dev/sdk";
import { z } from "zod";
import GoogleCalendarService from '@calcom/app-store/googlecalendar/lib'
export const create = schemaTask({
id: "calendar.google.create",
schema: z.object({
name: z.string(),
}),
run: async (payload: { name: string }) => {
const service = new GoogleCalendarService(logger)
service.create(payload)
},
});```
| "scripts": { | ||
| "lint": "eslint .", | ||
| "type-check": "tsc --noEmit", | ||
| "dev:trigger": "trigger dev", |
There was a problem hiding this comment.
dev trigger, this will start a local worker to tests tasks
| "lint": "eslint .", | ||
| "type-check": "tsc --noEmit", | ||
| "dev:trigger": "trigger dev", | ||
| "deploy:trigger": "trigger deploy" |
There was a problem hiding this comment.
this deploys our tasks to trigger.dev production env, we will need to have github actions for this
| default: { | ||
| maxAttempts: 3, | ||
| minTimeoutInMs: 1000, | ||
| maxTimeoutInMs: 10000, | ||
| factor: 2, | ||
| randomize: true, | ||
| }, |
There was a problem hiding this comment.
@keithwillcode let's look into what kind of config we need, this can also be overridenn on a per individual task basis
| primaryTasker: new GoogleCalendarTriggerDevTasker({ logger: logger as unknown as ILogger }), | ||
| fallbackTasker: new GoogleCalendarSyncTasker({ logger: logger as unknown as ILogger }), |
There was a problem hiding this comment.
we can handle which tasker is used using env variable, this ensures self-host can either implement their own third party tasker or stay with the synchronous tasker
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/v2/src/lib/logger.bridge.ts (1)
35-42: Guard env parsing: invalid LOGGER_BRIDGE_LOG_LEVEL currently disables all logging (NaN comparisons are always false).Clamp and validate the parsed level.
- private static readonly defaultSettings: IMyLoggerSettings = { - minLevel: process?.env?.LOGGER_BRIDGE_LOG_LEVEL - ? Number(process.env.LOGGER_BRIDGE_LOG_LEVEL) - : LogLevel.INFO, // Default to INFO level - displayTimestamp: true, - logFormat: "pretty", - }; + private static readonly defaultSettings: IMyLoggerSettings = { + minLevel: (() => { + const raw = process?.env?.LOGGER_BRIDGE_LOG_LEVEL; + const n = raw !== undefined ? Number(raw) : NaN; + return Number.isFinite(n) && n >= LogLevel.DEBUG && n <= LogLevel.ERROR + ? n + : LogLevel.INFO; // Default to INFO level + })(), + displayTimestamp: true, + logFormat: "pretty", + };
🧹 Nitpick comments (33)
packages/trigger/tsconfig.json (1)
3-4: Narrow the include for safer builds.
Including "." may pick up stray files (tests, temp). Consider limiting to source entries used by this package.- "include": ["."], + "include": ["src", "tasks", "index.ts", "trigger.config.ts"],packages/trigger/.gitignore (1)
1-1: Ignore typical build artifacts here (if not already covered at repo root).
If the root .gitignore doesn’t catch these, add them to avoid accidental commits..trigger +dist +build +node_modules +.turbo +*.tsbuildinfopackages/trigger/index.ts (1)
1-1: Consider limiting the public surface to a curated export list.
Prevents accidental API expansion when the SDK adds new exports; keeps downstream bundles lean.packages/trigger/package.json (1)
24-26: Redundant subpath export "./trigger".It resolves to the same file as "."; drop unless you truly need the alias.
- ".": "./index.ts", - "./trigger": "./index.ts" + ".": "./index.ts"packages/trigger/trigger.config.ts (3)
4-7: Prefer named export over default export.Aligns with repo guideline to avoid default exports.
-export default defineConfig({ +export const triggerConfig = defineConfig({ ... -}); +}); +export default triggerConfig; // if consumers still default-import; remove later
6-6: Project ID should be env-driven for multi-env setups.Hard-coding ties configs to one Trigger.dev project.
- project: "proj_klpqoyhxnkrxlceolbfy", // e.g., "proj_abc123" + project: process.env.TRIGGER_PROJECT ?? "proj_klpqoyhxnkrxlceolbfy",Optionally fail fast if missing in non-dev envs.
13-14: Comment contradicts value."Enable retries in development" but enabledInDev is false. Fix the comment or the flag.
- enabledInDev: false, // Enable retries in development + enabledInDev: false, // Disable retries in developmentpackages/features/tasker/types/logger.ts (3)
4-7: Model log level as a union instead of number for type-safety.-export interface IMyLoggerSettings { - minLevel: number; +export type LogLevel = "trace" | "debug" | "info" | "warn" | "error" | "fatal" | "silly"; +export interface IMyLoggerSettings { + minLevel: LogLevel; displayTimestamp: boolean; logFormat: "pretty" | "json" | "simple"; }
24-44: Prefer unknown[] over any[] for variadic args.Improves type safety without burdening callers.
- info(...args: any[]): void; - warn(...args: any[]): void; - error(...args: any[]): void; - debug(...args: any[]): void; - trace(...args: any[]): void; - fatal(...args: any[]): void; - silly(...args: any[]): void; + info(...args: unknown[]): void; + warn(...args: unknown[]): void; + error(...args: unknown[]): void; + debug(...args: unknown[]): void; + trace(...args: unknown[]): void; + fatal(...args: unknown[]): void; + silly(...args: unknown[]): void;
3-7: Naming nit: drop "My" from IMyLoggerSettings.Use LoggerSettings for clarity and consistency.
-export interface IMyLoggerSettings { +export interface LoggerSettings {Update the
ILogger.settingsaccordingly.apps/api/v2/src/lib/logger.bridge.ts (6)
60-65: Sub-logger should inherit settings and chain prefix.Right now it resets minLevel and drops the parent’s prefix/context.
- getSubLogger(options: { prefix?: string[] }): Logger { - const newLogger = new Logger(); - // Set the prefix for the *new* instance - newLogger.prefix = options?.prefix?.join(" ") ?? ""; - return newLogger; - } + getSubLogger(options: { prefix?: string[] }): Logger { + const newLogger = new Logger(this.settings); + const child = options?.prefix?.join(" ") ?? ""; + newLogger.prefix = [this.prefix, child].filter(Boolean).join(" ").trim(); + return newLogger; + }
69-99: Normalize level gating with a helper to avoid drift.Centralize the comparison logic and use level constants.
+ private shouldLog(level: number) { + const min = this.settings.minLevel; + return Number.isFinite(min) && min <= level; + } @@ - info(...args: any[]) { - this.settings.minLevel <= 1 && this.logInternal("log", ...args); - } + info(...args: any[]) { + this.shouldLog(LogLevel.INFO) && this.logInternal("log", ...args); + } @@ - warn(...args: any[]) { - this.settings.minLevel <= 2 && this.logInternal("warn", ...args); - } + warn(...args: any[]) { + this.shouldLog(LogLevel.WARN) && this.logInternal("warn", ...args); + } @@ - error(...args: any[]) { - this.settings.minLevel <= 3 && this.logInternal("error", ...args); - } + error(...args: any[]) { + this.shouldLog(LogLevel.ERROR) && this.logInternal("error", ...args); + } @@ - debug(...args: any[]) { - this.settings.minLevel === 0 && this.logInternal("debug", ...args); - } + debug(...args: any[]) { + this.shouldLog(LogLevel.DEBUG) && this.logInternal("debug", ...args); + } @@ - trace(...args: any[]) { - this.settings.minLevel === 0 && this.logInternal("verbose", ...args); - } + trace(...args: any[]) { + this.shouldLog(LogLevel.DEBUG) && this.logInternal("verbose", ...args); + } @@ - silly(...args: any[]) { - const sillyMessage = `silly: ${this.formatArgsAsString(args)}`; - this.settings.minLevel === 0 && this.logInternal("verbose", sillyMessage); - } + silly(...args: any[]) { + const sillyMessage = `silly: ${this.formatArgsAsString(args)}`; + this.shouldLog(LogLevel.DEBUG) && this.logInternal("verbose", sillyMessage); + }
102-117: Preserve Error details; JSON.stringify(new Error()) yields "{}".Special-case Error to include name/message/stack.
private formatArgsAsString(args: any[]): string { return args .map((arg) => { + if (arg instanceof Error) { + const stack = arg.stack ? `\n${arg.stack}` : ""; + return `${arg.name}: ${arg.message}${stack}`; + } if (typeof arg === "string") { return arg; }
118-146: Forward stack to NestLogger.error when available.Detect an Error arg and pass its stack to Nest logger.
- private logInternal(level: "log" | "warn" | "error" | "debug" | "verbose", ...args: any[]) { + private logInternal(level: "log" | "warn" | "error" | "debug" | "verbose", ...args: any[]) { try { - // Format message from potentially multiple arguments - const formattedMessage = this.formatArgsAsString(args); + const errObj = args.find((a) => a instanceof Error) as Error | undefined; + const printableArgs = errObj ? args.filter((a) => a !== errObj) : args; + const formattedMessage = this.formatArgsAsString(printableArgs); @@ switch (level) { case "log": this.nestLogger.log(message); break; case "warn": this.nestLogger.warn(message); break; case "error": - this.nestLogger.error(message); + this.nestLogger.error(message, errObj?.stack); break; case "debug": this.nestLogger.debug(message); break; case "verbose": this.nestLogger.verbose(message); break; } } catch (err) { - this.nestLogger.error( - `Could not bridge log message. Error: ${err instanceof Error ? err.message : String(err)}` - ); + const msg = `Could not bridge log message. Error: ${err instanceof Error ? err.message : String(err)}`; + this.nestLogger.error(msg, err instanceof Error ? err.stack : undefined); } }
3-10: Avoid duplicating types defined in features; import shared interfaces to prevent drift.If packages/features/tasker/types/logger.ts defines IMyLoggerSettings/ILogger, consider importing and implementing ILogger here to keep one source of truth.
25-26: Clarify DI plan for request context.This class is no longer an @Injectable. If you still inject it, ensure the provider is registered explicitly (useClass/useValue) and decide whether you need REQUEST scope for per-request metadata (e.g., requestId).
I can sketch the provider config (singleton vs. request-scoped) and a request-scoped wrapper if helpful.
packages/trigger/tasks/index.ts (1)
1-1: Prefer explicit re-exports over wildcard barrels.Prevents accidental API surface growth and improves tree-shaking.
-export * from "./calendars/create"; +export { create } from "./calendars/create";packages/features/tasker/types/tasker.ts (1)
3-5: Make dependencies immutableMark
loggeras readonly to prevent accidental reassignment and better convey intent.export interface ITaskerDependencies { - logger: ILogger; + readonly logger: ILogger; }packages/platform/libraries/vite.config.js (1)
196-196: Consider treating "@calcom/trigger" as externalIf
@calcom/triggeris a workspace package meant to be consumed separately, add it torollupOptions.externalto avoid bundling it into this lib. Add a corresponding entry inoutput.globalsonly if you ship a format requiring it.Example addition inside
rollupOptions.external:external: [ // ... "@calcom/trigger", ],apps/api/v2/src/lib/services/taskers/google-calendar-sync-tasker.service.ts (1)
1-13: File naming: avoid “.service.ts” for new filesGuideline suggests avoiding
.service.tsfor new files. Consider renaming togoogle-calendar-sync-tasker.tsand updating imports.apps/api/v2/src/lib/services/taskers/google-calendar-trigger-tasker.service.ts (1)
1-13: File naming: avoid “.service.ts” for new filesConsider
google-calendar-trigger-tasker.tsto align with the guideline and reduce Nest-style suffix proliferation.apps/api/v2/src/lib/modules/google-calendar-tasker.module.ts (1)
7-12: Optional: provide Logger via factory for configurable levelsIf you need env-driven log levels or prefixes, expose
LoggerwithuseFactory(injectingConfigService) instead of the class directly. Keeps the bridge decoupled from Nest while allowing per-env tuning.Example:
@Module({ providers: [ { provide: Logger, useFactory: (cfg: ConfigService) => new Logger({ minLevel: Number(cfg.get('LOGGER_BRIDGE_LOG_LEVEL') ?? 1) }), inject: [ConfigService], }, GoogleCalendarSyncTasker, GoogleCalendarTriggerDevTasker, GoogleCalendarTasker, ], exports: [GoogleCalendarTasker], }) export class GoogleCalendarTaskerModule {}packages/trigger/tasks/calendars/create.ts (1)
6-11: Tighten payload validation and fix minor log formatting.Enforce non-empty names and remove the extra space in the log message.
schema: z.object({ - name: z.string(), + name: z.string().min(1, "name is required"), }), run: async (payload: { name: string }) => { - logger.info(`Hello create ${payload.name}!`); + logger.info(`Hello create ${payload.name}!`); },packages/features/tasker/calendars/GoogleCalendarSyncTasker.ts (1)
9-18: Lower log verbosity and prefer structured logs to avoid leaking PII in info logs.Names/surnames can be PII; use debug and structured fields.
async createEvent(payload: Parameters<ICalendarTasker["createEvent"]>[0]) { - this.dependencies.logger.info(`Hello ${payload.name}`); + this.dependencies.logger.debug("createEvent", { name: payload.name }); const runId = `sync_${nanoid(10)}`; return { runId }; } async updateEvent(payload: Parameters<ICalendarTasker["updateEvent"]>[0]) { - this.dependencies.logger.info(`Hello ${payload.surname}`); + this.dependencies.logger.debug("updateEvent", { surname: payload.surname }); const runId = `sync_${nanoid(10)}`; return { runId }; }apps/api/v2/src/lib/services/taskers/google-calendar-tasker.service.ts (1)
16-20: Pass a sub-logger with a prefix for clearer log context.Improves log scanning and source attribution.
super({ primaryTasker: primaryTasker, fallbackTasker: fallbackTasker, - logger, + logger: logger.getSubLogger({ prefix: ["GoogleCalendarTasker"] }), });packages/features/tasker/calendars/GoogleCalendarTasker.ts (2)
3-11: Depend on the interface, not concrete implementations, to keep DI flexible.Loosen types so any ICalendarTasker can be wired as primary/fallback.
-import type { GoogleCalendarSyncTasker } from "./GoogleCalendarSyncTasker"; -import type { GoogleCalendarTriggerDevTasker } from "./GoogleCalendarTriggerTasker"; import type { ICalendarTasker } from "./calendar"; export interface ICalendarTaskerDependencies { - primaryTasker: GoogleCalendarTriggerDevTasker | GoogleCalendarSyncTasker; - fallbackTasker: GoogleCalendarSyncTasker; + primaryTasker: ICalendarTasker; + fallbackTasker: ICalendarTasker; logger: ILogger; }
21-30: Outdated comment path (“web/v2”).Adjust to the actual path (apps/api/v2) to avoid confusion.
- this is how we can now create the google calendar tasker, we will use dependency injection in web/v2 + this is how we can now create the google calendar tasker; in this repo we use dependency injection in apps/api/v2packages/features/tasker/calendars/GoogleCalendarTriggerTasker.ts (2)
17-19: Fix mislabeled log messages in updateEvent.They currently say "createEvent".
- this.dependencies.logger.debug("createEvent", payload); + this.dependencies.logger.debug("updateEvent", payload); - this.dependencies.logger.debug("createEvent handle id", handle.id); + this.dependencies.logger.debug("updateEvent handle id", handle.id);
9-14: createEvent path looks good; verify handle.id is a string.If handle.id isn’t guaranteed to be string, coerce to String to satisfy the interface.
- return { runId: handle.id }; + return { runId: String(handle.id) };packages/features/tasker/SafeDispatchTasker.ts (4)
33-36: Guard against non-function properties before .apply.Adds clearer error when an invalid key is dispatched.
- const method = this.primaryTasker[taskName] as (...args: any[]) => any; - return await method.apply(this.primaryTasker, args); + const method = this.primaryTasker[taskName] as unknown; + if (typeof method !== "function") { + throw new TypeError(`Task '${String(taskName)}' is not a function on primaryTasker.`); + } + return await (method as (...args: any[]) => any).apply(this.primaryTasker, args);
42-42: Log message should not assume a "SyncTasker" fallback.Generalize wording; fallback may not be sync.
- this.logger.warn(`Falling back to SyncTasker for '${String(taskName)}'.`); + this.logger.warn(`Falling back to fallback tasker for '${String(taskName)}'.`);
44-45: Prefer awaiting the fallback for symmetry and consistent stack traces.- const fallbackMethod = this.fallbackTasker[taskName] as (...args: any[]) => any; - return fallbackMethod.apply(this.fallbackTasker, args); + const fallbackMethod = this.fallbackTasker[taskName] as (...args: any[]) => any; + return await fallbackMethod.apply(this.fallbackTasker, args);
22-23: Consider lowering log level or redacting args.Dispatching with full args at info level may log PII (e.g., names). Use debug or redact sensitive fields.
- this.logger.info(`Dispatching task '${String(taskName)}'`, { args }); + this.logger.debug(`Dispatching task '${String(taskName)}'`, { args: "[redacted]" });
📜 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 (25)
apps/api/v2/package.json(1 hunks)apps/api/v2/src/lib/logger.bridge.ts(2 hunks)apps/api/v2/src/lib/modules/google-calendar-tasker.module.ts(1 hunks)apps/api/v2/src/lib/services/taskers/google-calendar-sync-tasker.service.ts(1 hunks)apps/api/v2/src/lib/services/taskers/google-calendar-tasker.service.ts(1 hunks)apps/api/v2/src/lib/services/taskers/google-calendar-trigger-tasker.service.ts(1 hunks)apps/api/v2/tsconfig.json(1 hunks)apps/web/package.json(1 hunks)packages/features/tasker/SafeDispatchTasker.ts(1 hunks)packages/features/tasker/calendars/GoogleCalendarSyncTasker.ts(1 hunks)packages/features/tasker/calendars/GoogleCalendarTasker.ts(1 hunks)packages/features/tasker/calendars/GoogleCalendarTriggerTasker.ts(1 hunks)packages/features/tasker/calendars/calendar.ts(1 hunks)packages/features/tasker/types/logger.ts(1 hunks)packages/features/tasker/types/tasker.ts(1 hunks)packages/platform/libraries/package.json(3 hunks)packages/platform/libraries/taskers.ts(1 hunks)packages/platform/libraries/vite.config.js(2 hunks)packages/trigger/.gitignore(1 hunks)packages/trigger/index.ts(1 hunks)packages/trigger/package.json(1 hunks)packages/trigger/tasks/calendars/create.ts(1 hunks)packages/trigger/tasks/index.ts(1 hunks)packages/trigger/trigger.config.ts(1 hunks)packages/trigger/tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{service,repository}.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/lib/services/taskers/google-calendar-trigger-tasker.service.tsapps/api/v2/src/lib/services/taskers/google-calendar-sync-tasker.service.tsapps/api/v2/src/lib/services/taskers/google-calendar-tasker.service.ts
**/*.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:
apps/api/v2/src/lib/services/taskers/google-calendar-trigger-tasker.service.tspackages/features/tasker/calendars/GoogleCalendarSyncTasker.tspackages/trigger/tasks/calendars/create.tsapps/api/v2/src/lib/services/taskers/google-calendar-sync-tasker.service.tspackages/trigger/trigger.config.tspackages/features/tasker/types/tasker.tspackages/trigger/index.tspackages/features/tasker/types/logger.tsapps/api/v2/src/lib/modules/google-calendar-tasker.module.tspackages/features/tasker/calendars/calendar.tspackages/features/tasker/calendars/GoogleCalendarTasker.tspackages/platform/libraries/taskers.tsapps/api/v2/src/lib/services/taskers/google-calendar-tasker.service.tspackages/features/tasker/calendars/GoogleCalendarTriggerTasker.tsapps/api/v2/src/lib/logger.bridge.tspackages/features/tasker/SafeDispatchTasker.tspackages/trigger/tasks/index.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:
apps/api/v2/src/lib/services/taskers/google-calendar-trigger-tasker.service.tspackages/features/tasker/calendars/GoogleCalendarSyncTasker.tspackages/trigger/tasks/calendars/create.tsapps/api/v2/src/lib/services/taskers/google-calendar-sync-tasker.service.tspackages/trigger/trigger.config.tspackages/features/tasker/types/tasker.tspackages/trigger/index.tspackages/features/tasker/types/logger.tsapps/api/v2/src/lib/modules/google-calendar-tasker.module.tspackages/features/tasker/calendars/calendar.tspackages/features/tasker/calendars/GoogleCalendarTasker.tspackages/platform/libraries/taskers.tsapps/api/v2/src/lib/services/taskers/google-calendar-tasker.service.tspackages/features/tasker/calendars/GoogleCalendarTriggerTasker.tsapps/api/v2/src/lib/logger.bridge.tspackages/features/tasker/SafeDispatchTasker.tspackages/trigger/tasks/index.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:
apps/api/v2/src/lib/services/taskers/google-calendar-trigger-tasker.service.tspackages/features/tasker/calendars/GoogleCalendarSyncTasker.tspackages/platform/libraries/vite.config.jspackages/trigger/tasks/calendars/create.tsapps/api/v2/src/lib/services/taskers/google-calendar-sync-tasker.service.tspackages/trigger/trigger.config.tspackages/features/tasker/types/tasker.tspackages/trigger/index.tspackages/features/tasker/types/logger.tsapps/api/v2/src/lib/modules/google-calendar-tasker.module.tspackages/features/tasker/calendars/calendar.tspackages/features/tasker/calendars/GoogleCalendarTasker.tspackages/platform/libraries/taskers.tsapps/api/v2/src/lib/services/taskers/google-calendar-tasker.service.tspackages/features/tasker/calendars/GoogleCalendarTriggerTasker.tsapps/api/v2/src/lib/logger.bridge.tspackages/features/tasker/SafeDispatchTasker.tspackages/trigger/tasks/index.ts
🧠 Learnings (1)
📚 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/features/tasker/calendars/calendar.ts
🧬 Code graph analysis (9)
apps/api/v2/src/lib/services/taskers/google-calendar-trigger-tasker.service.ts (4)
apps/api/v2/src/lib/services/taskers/google-calendar-sync-tasker.service.ts (1)
Injectable(6-13)apps/api/v2/src/lib/services/taskers/google-calendar-tasker.service.ts (1)
Injectable(9-22)packages/features/tasker/calendars/GoogleCalendarTriggerTasker.ts (1)
GoogleCalendarTriggerDevTasker(6-22)apps/api/v2/src/lib/logger.bridge.ts (1)
Logger(26-152)
packages/features/tasker/calendars/GoogleCalendarSyncTasker.ts (2)
packages/features/tasker/calendars/calendar.ts (1)
ICalendarTasker(1-4)packages/features/tasker/types/tasker.ts (1)
ITaskerDependencies(3-5)
apps/api/v2/src/lib/services/taskers/google-calendar-sync-tasker.service.ts (4)
apps/api/v2/src/lib/services/taskers/google-calendar-tasker.service.ts (1)
Injectable(9-22)apps/api/v2/src/lib/services/taskers/google-calendar-trigger-tasker.service.ts (1)
Injectable(6-13)packages/features/tasker/calendars/GoogleCalendarSyncTasker.ts (1)
GoogleCalendarSyncTasker(6-19)apps/api/v2/src/lib/logger.bridge.ts (1)
Logger(26-152)
packages/features/tasker/types/tasker.ts (1)
packages/features/tasker/types/logger.ts (1)
ILogger(11-44)
apps/api/v2/src/lib/modules/google-calendar-tasker.module.ts (4)
packages/features/tasker/calendars/GoogleCalendarSyncTasker.ts (1)
GoogleCalendarSyncTasker(6-19)packages/features/tasker/calendars/GoogleCalendarTriggerTasker.ts (1)
GoogleCalendarTriggerDevTasker(6-22)apps/api/v2/src/lib/logger.bridge.ts (1)
Logger(26-152)packages/features/tasker/calendars/GoogleCalendarTasker.ts (1)
GoogleCalendarTasker(15-19)
packages/features/tasker/calendars/GoogleCalendarTasker.ts (4)
packages/features/tasker/calendars/GoogleCalendarTriggerTasker.ts (1)
GoogleCalendarTriggerDevTasker(6-22)packages/features/tasker/calendars/GoogleCalendarSyncTasker.ts (1)
GoogleCalendarSyncTasker(6-19)packages/features/tasker/types/logger.ts (1)
ILogger(11-44)packages/features/tasker/calendars/calendar.ts (1)
ICalendarTasker(1-4)
apps/api/v2/src/lib/services/taskers/google-calendar-tasker.service.ts (3)
apps/api/v2/src/lib/services/taskers/google-calendar-sync-tasker.service.ts (1)
Injectable(6-13)apps/api/v2/src/lib/services/taskers/google-calendar-trigger-tasker.service.ts (1)
Injectable(6-13)apps/api/v2/src/lib/logger.bridge.ts (1)
Logger(26-152)
packages/features/tasker/calendars/GoogleCalendarTriggerTasker.ts (3)
packages/features/tasker/calendars/calendar.ts (1)
ICalendarTasker(1-4)packages/features/tasker/types/tasker.ts (1)
ITaskerDependencies(3-5)packages/trigger/tasks/calendars/create.ts (1)
create(4-12)
packages/features/tasker/SafeDispatchTasker.ts (1)
packages/features/tasker/types/logger.ts (1)
ILogger(11-44)
⏰ 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 (14)
packages/trigger/tsconfig.json (1)
1-5: Config looks fine and consistent with the monorepo base.
Extends base, scopes includes/excludes sensibly.apps/web/package.json (1)
44-45: No imports of @calcom/trigger found – no bundling concerns
Audit across apps/web (static import, dynamic import, require) returned zero matches; safe to leave the dependency as-is.packages/trigger/index.ts (1)
1-1: Good: named re-exports promote tree-shaking and clear imports.
No default export; aligns with guidelines.packages/trigger/package.json (1)
10-11: CLI availability check."trigger dev/deploy" require the CLI. You have trigger.dev in devDependencies—good. Ensure CI env installs dev deps where these scripts run.
Also applies to: 20-20
packages/platform/libraries/package.json (1)
3-3: Placeholder version "9.9.9".If this is a real publish, set a semver that matches your release process; otherwise ensure dependants pin correctly.
packages/trigger/trigger.config.ts (1)
26-31: No action needed; Prisma schema path resolves correctlyThe relative path
"../prisma/schema.prisma"frompackages/trigger/trigger.config.tspoints topackages/prisma/schema.prisma, matching the located schema file—no changes required.apps/api/v2/tsconfig.json (1)
15-33: Taskers alias resolution verified
All imports of@calcom/platform-libraries/taskersinapps/api/v2use the tsconfig path alias, no deep relative imports remain, and the./taskerssubpath is exported inpackages/platform/libraries/package.json.packages/platform/libraries/vite.config.js (1)
31-31: Packaging fortaskersentry verified:taskers.tsexists,package.jsonexports include"./taskers"with the correcttypespath (dist/taskers.d.ts), the file re-exports the expected symbols, and the tsconfig path alias is set. Run your build to producedist/taskers.d.tsbefore publishing.apps/api/v2/src/lib/services/taskers/google-calendar-sync-tasker.service.ts (1)
6-13: LGTM: clean DI wrapper over base taskerConstructor cleanly forwards the logger; extends base correctly.
apps/api/v2/src/lib/services/taskers/google-calendar-trigger-tasker.service.ts (1)
6-13: LGTM: consistent DI for dev trigger taskerMatches the sync tasker pattern; forwards logger properly.
packages/platform/libraries/taskers.ts (1)
1-3: Barrel looks good; named exports only.Centralized, named re-exports aid tree-shaking and refactors.
apps/api/v2/src/lib/services/taskers/google-calendar-tasker.service.ts (1)
1-22: Ignore file naming suggestion;.service.tssuffix aligns with the existing pattern across apps/api/v2.Likely an incorrect or invalid review comment.
packages/features/tasker/calendars/GoogleCalendarTasker.ts (1)
13-13: Verify TASKS consumption for dispatch gating
ICalendarTasker defines updateEvent but TASKS only lists"createEvent". If TASKS is used to whitelist dispatch names, include"updateEvent":-export const TASKS = ["createEvent"] as const; +export const TASKS = ["createEvent", "updateEvent"] as const;Otherwise confirm TASKS isn’t consumed elsewhere before changing.
packages/features/tasker/SafeDispatchTasker.ts (1)
5-16: Solid base with typed dispatch and DI.Type constraints and dependency wiring look correct.
| "@calcom/platform-libraries/private-links": ["../../../packages/platform/libraries/private-links.ts"], | ||
| "@calcom/platform-libraries/taskers": ["../../../packages/platform/libraries/taskers.ts"] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Path alias overrides the package export; remove it to avoid split sources.
@calcom/platform-libraries now exports "taskers". Keeping this alias points imports to a raw TS file and can desync with the published package. Remove the alias and fix the dangling comma.
- "@calcom/platform-libraries/private-links": ["../../../packages/platform/libraries/private-links.ts"],
- "@calcom/platform-libraries/taskers": ["../../../packages/platform/libraries/taskers.ts"]
+ "@calcom/platform-libraries/private-links": ["../../../packages/platform/libraries/private-links.ts"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@calcom/platform-libraries/private-links": ["../../../packages/platform/libraries/private-links.ts"], | |
| "@calcom/platform-libraries/taskers": ["../../../packages/platform/libraries/taskers.ts"] | |
| "@calcom/platform-libraries/private-links": ["../../../packages/platform/libraries/private-links.ts"] |
🤖 Prompt for AI Agents
In apps/api/v2/tsconfig.json around lines 31 to 32, the path alias for
"@calcom/platform-libraries/taskers" overrides the package export and points to
a raw TS file; remove the entire "@calcom/platform-libraries/taskers" alias
entry from the paths section so imports resolve to the packaged export instead,
and after removing the line ensure the JSON stays valid by deleting the
now-dangling trailing comma (or adjusting commas appropriately).
| createEvent(payload: { name: string }): Promise<{ runId: string }>; | ||
| updateEvent(payload: { surname: string }): Promise<{ runId: string }>; |
There was a problem hiding this comment.
Inconsistent payload shapes ("name" vs "surname") will break implementations.
Likely a typo; align both methods and consider real event fields.
export interface ICalendarTasker {
- createEvent(payload: { name: string }): Promise<{ runId: string }>;
- updateEvent(payload: { surname: string }): Promise<{ runId: string }>;
+ createEvent(payload: { name: string }): Promise<{ runId: string }>;
+ updateEvent(payload: { name: string }): Promise<{ runId: string }>;
}Optionally, introduce domain types (calendarId, eventId, start/end, summary) and reuse across taskers.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| createEvent(payload: { name: string }): Promise<{ runId: string }>; | |
| updateEvent(payload: { surname: string }): Promise<{ runId: string }>; | |
| export interface ICalendarTasker { | |
| createEvent(payload: { name: string }): Promise<{ runId: string }>; | |
| updateEvent(payload: { name: string }): Promise<{ runId: string }>; | |
| } |
🤖 Prompt for AI Agents
In packages/features/tasker/calendars/calendar.ts around lines 2-3, the method
payloads are inconsistent: createEvent expects { name: string } while
updateEvent expects { surname: string }; change them to use the same field
(prefer a semantically clear field like "summary" or "title") and update both
signatures to the same payload shape, and replace ad-hoc objects with a shared
EventPayload type (and optionally richer domain types such as CalendarId,
EventId, start/end timestamps) so both methods reuse the same definition.
| "main": "./index.ts", | ||
| "types": "./index.ts", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid shipping .ts entry points; map exports to built JS + d.ts.
Pointing "main/types" and "exports" to .ts will break Node/bundlers outside your Vite build. Compile and export dist artifacts instead.
Apply after adding a build that outputs to dist:
- "main": "./index.ts",
- "types": "./index.ts",
+ "type": "module",
+ "main": "./dist/index.js",
+ "types": "./dist/index.d.ts",
...
- "exports": {
- ".": "./index.ts",
- "./trigger": "./index.ts"
- }
+ "exports": {
+ ".": {
+ "import": "./dist/index.js",
+ "types": "./dist/index.d.ts"
+ }
+ }If this package is purely internal and only resolved via a Vite alias, confirm no runtime imports rely on Node resolution.
Also applies to: 23-26
🏁 Script executed:
#!/bin/bash
# Find runtime imports of @calcom/trigger (ESM/CJS)
rg -nP --glob '!**/dist/**' "(from|require\\()\\s*['\"]@calcom/trigger" -C2Length of output: 407
Map package entrypoints to compiled dist artifacts
Runtime imports (@calcom/trigger) still point at .ts (e.g. packages/features/tasker/calendars/GoogleCalendarTriggerTasker.ts). After adding a build that outputs to dist, update packages/trigger/package.json:
- "main": "./index.ts",
- "types": "./index.ts",
+ "type": "module",
+ "main": "./dist/index.js",
+ "types": "./dist/index.d.ts",
…
- "exports": {
- ".": "./index.ts",
- "./trigger": "./index.ts"
- }
+ "exports": {
+ ".": {
+ "import": "./dist/index.js",
+ "types": "./dist/index.d.ts"
+ }
+ }Ensure no runtime imports rely on .ts entrypoints.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/trigger/package.json around lines 5-6, the package main and types
fields point to TypeScript sources ("./index.ts") instead of compiled artifacts;
update "main" to "./dist/index.js" and "types" to "./dist/index.d.ts" (or the
actual emitted JS and d.ts paths), run the build to produce dist, then search
the repo for any runtime imports that reference .ts entrypoints (e.g.
packages/features/tasker/calendars/GoogleCalendarTriggerTasker.ts) and replace
them with the corresponding package import (@calcom/trigger) or the compiled
paths so no runtime code imports .ts files; finally verify package.json "files"
includes dist and that consumers resolve the new entrypoints.
| @Injectable() | ||
| export class GoogleCalendarTasker extends BaseGoogleCalendarTasker { | ||
| constructor( | ||
| primaryTasker: GoogleCalendarTriggerDevTasker, |
There was a problem hiding this comment.
for example here:
ApiV2 could provide it's own BullMQ tasker
There was a problem hiding this comment.
and this could also be controller by env variable
| "@calcom/platform-constants": "*", | ||
| "@calcom/platform-enums": "*", | ||
| "@calcom/platform-libraries": "npm:@calcom/platform-libraries@0.0.335", | ||
| "@calcom/platform-libraries": "npm:@calcom/platform-libraries@9.9.9", |
There was a problem hiding this comment.
publish libraries before merge
| { | ||
| "name": "@calcom/platform-libraries", | ||
| "version": "0.0.0", | ||
| "version": "9.9.9", |
There was a problem hiding this comment.
publish libraries before merging
| import { z } from "zod"; | ||
|
|
||
| export const create = schemaTask({ | ||
| id: "calendar.google.create.event", |
There was a problem hiding this comment.
I think we can change this namespace...
suggestion: calendar.google.create-event or calendar.google.event.create
create as part of namespace can lead confusing, I think.
| import { z } from "zod"; | ||
|
|
||
| export const create = schemaTask({ | ||
| id: "calendar.google.create.event", |
There was a problem hiding this comment.
.event might be overkill and not needed
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
ThyMinimalDev
changed the title
…ger dependency - Add DI tokens for GoogleCalendarTasker, sync/trigger taskers, and logger - Create LoggerAdapter to bridge tslog to ILogger interface - Create modules for GoogleCalendarTasker, GoogleCalendarSyncTasker, GoogleCalendarTriggerDevTasker, and Logger - Create container that wires all dependencies together - Add index file for easy imports - Follow existing @evyweb/ioctopus patterns from BusyTimes and other services Co-Authored-By: morgan@cal.com <morgan@cal.com>
Co-Authored-By: morgan@cal.com <morgan@cal.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
packages/lib/di/tokens.ts (1)
54-61: New DI tokens look good; consider freezing for safety.Nice separation between service and module tokens. To prevent accidental mutation at runtime and improve type safety, consider freezing and/or asserting const.
Apply:
export const DI_TOKENS = { ... GOOGLE_CALENDAR_TRIGGER_DEV_TASKER: Symbol("GoogleCalendarTriggerDevTasker"), GOOGLE_CALENDAR_TRIGGER_DEV_TASKER_MODULE: Symbol("GoogleCalendarTriggerDevTaskerModule"), -}; +} as const; +Object.freeze(DI_TOKENS);packages/lib/di/modules/Logger.ts (1)
1-7: Optional: allow configurable settings via DI instead of env-only.If you anticipate per-context logging (tests, workers), inject settings via a token to avoid hard-coding env lookups in the adapter.
Example:
+import type { IMyLoggerSettings } from "../adapters/LoggerAdapter"; +const LOGGER_SETTINGS = Symbol("LoggerSettings"); export const loggerModule = createModule(); -loggerModule.bind(DI_TOKENS.LOGGER).toClass(LoggerAdapter); +loggerModule.bind(LOGGER_SETTINGS).toValue({ /* defaults */ } satisfies IMyLoggerSettings); +loggerModule.bind(DI_TOKENS.LOGGER).toClass(LoggerAdapter, { + settings: LOGGER_SETTINGS, +});(Requires
LoggerAdapterto acceptsettingsin its constructor.)packages/lib/di/containers/GoogleCalendarTasker.ts (1)
11-19: Prefer lazy container init + reset hook for tests.A top-level singleton container can be hard to isolate in tests and can capture stale env/config. Lazy-init and a test-only reset improve ergonomics without affecting prod.
Apply:
-import { createContainer } from "@evyweb/ioctopus"; +import { createContainer, type Container } from "@evyweb/ioctopus"; @@ -const container = createContainer(); -container.load(DI_TOKENS.LOGGER_MODULE, loggerModule); -container.load(DI_TOKENS.GOOGLE_CALENDAR_SYNC_TASKER_MODULE, googleCalendarSyncTaskerModule); -container.load(DI_TOKENS.GOOGLE_CALENDAR_TRIGGER_DEV_TASKER_MODULE, googleCalendarTriggerDevTaskerModule); -container.load(DI_TOKENS.GOOGLE_CALENDAR_TASKER_MODULE, googleCalendarTaskerModule); +let container: Container | null = null; +function ensureContainer() { + if (container) return container; + const c = createContainer(); + c.load(DI_TOKENS.LOGGER_MODULE, loggerModule); + c.load(DI_TOKENS.GOOGLE_CALENDAR_SYNC_TASKER_MODULE, googleCalendarSyncTaskerModule); + c.load(DI_TOKENS.GOOGLE_CALENDAR_TRIGGER_DEV_TASKER_MODULE, googleCalendarTriggerDevTaskerModule); + c.load(DI_TOKENS.GOOGLE_CALENDAR_TASKER_MODULE, googleCalendarTaskerModule); + container = c; + return c; +} @@ export function getGoogleCalendarTasker() { - return container.get<GoogleCalendarTasker>(DI_TOKENS.GOOGLE_CALENDAR_TASKER); + return ensureContainer().get<GoogleCalendarTasker>(DI_TOKENS.GOOGLE_CALENDAR_TASKER); } + +// Test-only helper +export function __resetGoogleCalendarTaskerContainerForTests() { + container = null; +}packages/lib/di/modules/GoogleCalendarSyncTasker.ts (1)
1-11: Nit: make the dependency map immutable.Small safety to avoid accidental reassignment.
Apply:
-googleCalendarSyncTaskerModule.bind(DI_TOKENS.GOOGLE_CALENDAR_SYNC_TASKER).toClass(GoogleCalendarSyncTasker, { - logger: DI_TOKENS.LOGGER, -} satisfies Record<keyof ITaskerDependencies, symbol>); +googleCalendarSyncTaskerModule + .bind(DI_TOKENS.GOOGLE_CALENDAR_SYNC_TASKER) + .toClass( + GoogleCalendarSyncTasker, + { + logger: DI_TOKENS.LOGGER, + } as const satisfies Record<keyof ITaskerDependencies, symbol>, + );packages/lib/di/modules/GoogleCalendarTasker.ts (1)
10-12: Avoid shipping the dev trigger as primary in production. Gate selection by env.Make the primary tasker conditional to prevent accidental use of the dev trigger in prod.
Apply:
import { DI_TOKENS } from "../tokens"; export const googleCalendarTaskerModule = createModule(); +const primaryToken = + process.env.NODE_ENV === "production" + ? DI_TOKENS.GOOGLE_CALENDAR_SYNC_TASKER + : DI_TOKENS.GOOGLE_CALENDAR_TRIGGER_DEV_TASKER; googleCalendarTaskerModule.bind(DI_TOKENS.GOOGLE_CALENDAR_TASKER).toClass(GoogleCalendarTasker, { - primaryTasker: DI_TOKENS.GOOGLE_CALENDAR_TRIGGER_DEV_TASKER, + primaryTasker: primaryToken, fallbackTasker: DI_TOKENS.GOOGLE_CALENDAR_SYNC_TASKER, logger: DI_TOKENS.LOGGER, } satisfies Record<keyof ICalendarTaskerDependencies, symbol>);packages/lib/di/adapters/LoggerAdapter.ts (3)
6-10: Harden env parsing and clamp log level.Add radix, handle NaN, and clamp to an expected range.
- readonly settings: IMyLoggerSettings = { - minLevel: parseInt(process.env.NEXT_PUBLIC_LOGGER_LEVEL || "4"), + readonly settings: IMyLoggerSettings = { + minLevel: (() => { + const raw = process.env.NEXT_PUBLIC_LOGGER_LEVEL; + const n = Number.parseInt(raw ?? "", 10); + return Number.isNaN(n) ? 4 : Math.min(Math.max(n, 0), 6); + })(), displayTimestamp: true, logFormat: process.env.NODE_ENV === "production" ? "json" : "pretty", };
29-55: Consider tightening arg types for log methods.Using unknown[] (or overloads for string | object | Error) improves type-safety without affecting usage.
- info(...args: any[]): void { + info(...args: unknown[]): void { logger.info(...args); } - warn(...args: any[]): void { + warn(...args: unknown[]): void { logger.warn(...args); } - error(...args: any[]): void { + error(...args: unknown[]): void { logger.error(...args); } - debug(...args: any[]): void { + debug(...args: unknown[]): void { logger.debug(...args); } - trace(...args: any[]): void { + trace(...args: unknown[]): void { logger.trace(...args); } - fatal(...args: any[]): void { + fatal(...args: unknown[]): void { logger.fatal(...args); } - silly(...args: any[]): void { + silly(...args: unknown[]): void { logger.silly(...args); }
3-3: Migrate logger to a named export for consistencypackages/lib/logger.ts currently uses
export default logger; refactor it toexport const logger = new Logger(/*…*/)and update consumers (e.g. in packages/lib/di/adapters/LoggerAdapter.ts) to
import { logger } from '../../logger';
📜 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 (8)
packages/lib/di/adapters/LoggerAdapter.ts(1 hunks)packages/lib/di/containers/GoogleCalendarTasker.ts(1 hunks)packages/lib/di/index.ts(1 hunks)packages/lib/di/modules/GoogleCalendarSyncTasker.ts(1 hunks)packages/lib/di/modules/GoogleCalendarTasker.ts(1 hunks)packages/lib/di/modules/GoogleCalendarTriggerDevTasker.ts(1 hunks)packages/lib/di/modules/Logger.ts(1 hunks)packages/lib/di/tokens.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/lib/di/tokens.tspackages/lib/di/modules/GoogleCalendarTriggerDevTasker.tspackages/lib/di/modules/GoogleCalendarSyncTasker.tspackages/lib/di/modules/GoogleCalendarTasker.tspackages/lib/di/index.tspackages/lib/di/modules/Logger.tspackages/lib/di/containers/GoogleCalendarTasker.tspackages/lib/di/adapters/LoggerAdapter.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/lib/di/tokens.tspackages/lib/di/modules/GoogleCalendarTriggerDevTasker.tspackages/lib/di/modules/GoogleCalendarSyncTasker.tspackages/lib/di/modules/GoogleCalendarTasker.tspackages/lib/di/index.tspackages/lib/di/modules/Logger.tspackages/lib/di/containers/GoogleCalendarTasker.tspackages/lib/di/adapters/LoggerAdapter.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/lib/di/tokens.tspackages/lib/di/modules/GoogleCalendarTriggerDevTasker.tspackages/lib/di/modules/GoogleCalendarSyncTasker.tspackages/lib/di/modules/GoogleCalendarTasker.tspackages/lib/di/index.tspackages/lib/di/modules/Logger.tspackages/lib/di/containers/GoogleCalendarTasker.tspackages/lib/di/adapters/LoggerAdapter.ts
🧬 Code graph analysis (6)
packages/lib/di/modules/GoogleCalendarTriggerDevTasker.ts (2)
packages/lib/di/tokens.ts (1)
DI_TOKENS(1-62)packages/features/tasker/types/tasker.ts (1)
ITaskerDependencies(3-5)
packages/lib/di/modules/GoogleCalendarSyncTasker.ts (2)
packages/lib/di/tokens.ts (1)
DI_TOKENS(1-62)packages/features/tasker/types/tasker.ts (1)
ITaskerDependencies(3-5)
packages/lib/di/modules/GoogleCalendarTasker.ts (2)
packages/lib/di/tokens.ts (1)
DI_TOKENS(1-62)packages/features/tasker/calendars/GoogleCalendarTasker.ts (1)
ICalendarTaskerDependencies(7-11)
packages/lib/di/modules/Logger.ts (2)
packages/lib/di/tokens.ts (1)
DI_TOKENS(1-62)packages/lib/di/adapters/LoggerAdapter.ts (1)
LoggerAdapter(5-56)
packages/lib/di/containers/GoogleCalendarTasker.ts (6)
packages/lib/di/tokens.ts (1)
DI_TOKENS(1-62)packages/lib/di/modules/Logger.ts (1)
loggerModule(6-6)packages/lib/di/modules/GoogleCalendarSyncTasker.ts (1)
googleCalendarSyncTaskerModule(8-8)packages/lib/di/modules/GoogleCalendarTriggerDevTasker.ts (1)
googleCalendarTriggerDevTaskerModule(8-8)packages/lib/di/modules/GoogleCalendarTasker.ts (1)
googleCalendarTaskerModule(8-8)packages/lib/di/index.ts (1)
getGoogleCalendarTasker(1-1)
packages/lib/di/adapters/LoggerAdapter.ts (1)
packages/features/tasker/types/logger.ts (2)
ILogger(11-44)IMyLoggerSettings(3-7)
⏰ 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 (8)
packages/lib/di/modules/Logger.ts (1)
6-7: LGTM: clean DI binding for ILogger.Binding
DI_TOKENS.LOGGERtoLoggerAdaptervia a module is straightforward and keeps consumers decoupled.packages/lib/di/index.ts (1)
1-2: LGTM: re-exports are explicit and named.Keeps tree-shaking friendly and aligns with the no-default-exports guideline.
packages/lib/di/containers/GoogleCalendarTasker.ts (1)
5-15: Confirm binding and import/export for GOOGLE_CALENDAR_TASKER
Binding forDI_TOKENS.GOOGLE_CALENDAR_TASKERis present in packages/lib/di/modules/GoogleCalendarTasker.ts (line 9); ensure thatGoogleCalendarTaskeris imported at the top of this file and that its defining module exports the class.packages/lib/di/modules/GoogleCalendarSyncTasker.ts (1)
8-11: LGTM: binding with typed dependency map is solid.Using
satisfies Record<keyof ITaskerDependencies, symbol>ensures the DI map stays in sync with constructor needs.packages/lib/di/modules/GoogleCalendarTasker.ts (2)
8-13: Type-safe DI binding looks good.Binding GoogleCalendarTasker with explicit tokens and the satisfies constraint is clean and future-proof.
8-13: DI load order verified
Sync and TriggerDev modules are loaded before the Tasker module in the GoogleCalendarTasker container.packages/lib/di/modules/GoogleCalendarTriggerDevTasker.ts (2)
8-13: Binding and type constraint look correct.Token mapping to logger via ITaskerDependencies is consistent with the DI pattern used elsewhere.
3-12: Verified named export:GoogleCalendarTriggerDevTaskeris correctly exported from@calcom/features/tasker/calendars/GoogleCalendarTriggerTasker.
| export class LoggerAdapter implements ILogger { | ||
| readonly settings: IMyLoggerSettings = { | ||
| minLevel: parseInt(process.env.NEXT_PUBLIC_LOGGER_LEVEL || "4"), | ||
| displayTimestamp: true, | ||
| logFormat: process.env.NODE_ENV === "production" ? "json" : "pretty", | ||
| }; | ||
|
|
||
| getSubLogger(options: { prefix?: string[] }): ILogger { | ||
| const adapter = new LoggerAdapter(); | ||
| const originalLogger = logger; | ||
|
|
||
| const prefix = options.prefix ? `[${options.prefix.join(":")}] ` : ""; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Sub-logger drops parent prefix; compose prefixes for nested contexts.
Carry forward existing prefix when chaining getSubLogger.
export class LoggerAdapter implements ILogger {
+ private readonly prefixParts: string[];
+ constructor(prefix: string[] = []) {
+ this.prefixParts = prefix;
+ }
readonly settings: IMyLoggerSettings = {
@@
- getSubLogger(options: { prefix?: string[] }): ILogger {
- const adapter = new LoggerAdapter();
+ getSubLogger(options: { prefix?: string[] }): ILogger {
+ const combined = [...this.prefixParts, ...(options.prefix ?? [])];
+ const adapter = new LoggerAdapter(combined);
const originalLogger = logger;
-
- const prefix = options.prefix ? `[${options.prefix.join(":")}] ` : "";
+ const prefix = combined.length ? `[${combined.join(":")}] ` : "";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export class LoggerAdapter implements ILogger { | |
| readonly settings: IMyLoggerSettings = { | |
| minLevel: parseInt(process.env.NEXT_PUBLIC_LOGGER_LEVEL || "4"), | |
| displayTimestamp: true, | |
| logFormat: process.env.NODE_ENV === "production" ? "json" : "pretty", | |
| }; | |
| getSubLogger(options: { prefix?: string[] }): ILogger { | |
| const adapter = new LoggerAdapter(); | |
| const originalLogger = logger; | |
| const prefix = options.prefix ? `[${options.prefix.join(":")}] ` : ""; | |
| export class LoggerAdapter implements ILogger { | |
| private readonly prefixParts: string[]; | |
| constructor(prefix: string[] = []) { | |
| this.prefixParts = prefix; | |
| } | |
| readonly settings: IMyLoggerSettings = { | |
| minLevel: parseInt(process.env.NEXT_PUBLIC_LOGGER_LEVEL || "4"), | |
| displayTimestamp: true, | |
| logFormat: process.env.NODE_ENV === "production" ? "json" : "pretty", | |
| }; | |
| getSubLogger(options: { prefix?: string[] }): ILogger { | |
| const combined = [...this.prefixParts, ...(options.prefix ?? [])]; | |
| const adapter = new LoggerAdapter(combined); | |
| const originalLogger = logger; | |
| const prefix = combined.length ? `[${combined.join(":")}] ` : ""; | |
| // …rest of method… |
🤖 Prompt for AI Agents
In packages/lib/di/adapters/LoggerAdapter.ts around lines 5 to 17, getSubLogger
currently discards any existing parent prefix when creating a nested logger;
change it to compose prefixes instead: read the parent prefix from the original
logger (if any), combine it with the new options.prefix (joining with ":" and
wrapping in brackets only once), assign the composed prefix to the new adapter
without mutating the parent, copy over relevant settings/context from the
original logger to the new adapter, and return the new adapter so nested
contexts preserve and extend the parent prefix.
| @@ -0,0 +1,13 @@ | |||
| import { createModule } from "@evyweb/ioctopus"; | |||
|
|
|||
| import { GoogleCalendarTriggerDevTasker } from "@calcom/features/tasker/calendars/GoogleCalendarTriggerTasker"; | |||
There was a problem hiding this comment.
Fix import path/class mismatch (will fail at build).
Imported name is GoogleCalendarTriggerDevTasker, but the path ends with GoogleCalendarTriggerTasker. Align the path.
-import { GoogleCalendarTriggerDevTasker } from "@calcom/features/tasker/calendars/GoogleCalendarTriggerTasker";
+import { GoogleCalendarTriggerDevTasker } from "@calcom/features/tasker/calendars/GoogleCalendarTriggerDevTasker";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { GoogleCalendarTriggerDevTasker } from "@calcom/features/tasker/calendars/GoogleCalendarTriggerTasker"; | |
| import { GoogleCalendarTriggerDevTasker } from "@calcom/features/tasker/calendars/GoogleCalendarTriggerDevTasker"; |
🤖 Prompt for AI Agents
In packages/lib/di/modules/GoogleCalendarTriggerDevTasker.ts around line 3, the
import statement uses the identifier GoogleCalendarTriggerDevTasker but imports
from a path ending with GoogleCalendarTriggerTasker, causing a mismatch and
build failure; change the import path or the exported class name so they match
(either import from
"@calcom/features/tasker/calendars/GoogleCalendarTriggerDevTasker" if that
file/class exists, or import the correct exported name
GoogleCalendarTriggerTasker if you intended that class), and update any
downstream references to use the matching identifier.
- Add comprehensive build method that reconstructs GoogleCalendarEvent from minimal ID-based parameters - Support team events with COLLECTIVE and ROUND_ROBIN scheduling types - Handle delegation credentials with organization membership verification - Add create method that mirrors GoogleCalendarService.createEvent implementation - Include buildAndCreate convenience method for combined operations - Support complex scenarios: team events, delegation credentials, dynamic destination calendar selection - Generate rich calendar descriptions using getRichDescription - Maintain compatibility with existing Cal.com event construction patterns Co-Authored-By: morgan@cal.com <morgan@cal.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/features/tasker/calendars/GoogleCalendarProcessor.ts (3)
127-131: Avoid extra DB hit for team.parentId.You already select parentId when loading team; reuse it instead of a second prisma.team.findUnique later.
Apply:
let teamMembers: any[] = []; let teamDestinationCalendars: any[] = []; let teamObject: CalendarEvent["team"] = undefined; + let teamParentId: number | null = null; @@ - const allTeamUsers = team.members.map((member) => member.user); + teamParentId = team.parentId; + const allTeamUsers = team.members.map((member) => member.user); @@ - const organizerOrganizationId = organizerUser.profiles[0]?.organizationId; - const teamParentId = teamObject?.id - ? (await prisma.team.findUnique({ where: { id: teamObject.id }, select: { parentId: true } }))?.parentId - : null; + const organizerOrganizationId = organizerUser.profiles[0]?.organizationId;Then the existing getBookerBaseUrl call uses teamParentId as-is.
Also applies to: 158-171, 285-291
410-416: Use centralized logger instead of console.error.This PR introduces a LoggerAdapter/DI. Prefer logger.error with structured fields for consistency and redaction controls.
Want me to wire ILogger into this class constructor and swap the call?
127-130: Type the arrays (avoid any[]).Provide concrete types for teamMembers and teamDestinationCalendars to improve safety and editor support.
📜 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/features/tasker/calendars/GoogleCalendarProcessor.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/features/tasker/calendars/GoogleCalendarProcessor.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/features/tasker/calendars/GoogleCalendarProcessor.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/features/tasker/calendars/GoogleCalendarProcessor.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-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/features/tasker/calendars/GoogleCalendarProcessor.ts
🧬 Code graph analysis (1)
packages/features/tasker/calendars/GoogleCalendarProcessor.ts (6)
packages/types/Calendar.d.ts (2)
CalendarEvent(163-226)NewCalendarEventType(71-84)packages/lib/timeFormat.ts (1)
getTimeFormatStringFromUserTimeFormat(27-29)packages/lib/event.ts (1)
getEventName(31-113)packages/lib/EventManager.ts (1)
processLocation(99-112)packages/lib/CalEventParser.ts (1)
getRichDescription(375-404)packages/app-store/googlecalendar/lib/CalendarService.ts (1)
GoogleCalendarService(56-1138)
⏰ 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/features/tasker/calendars/GoogleCalendarProcessor.ts (3)
6-6: Likely wrong import path for processLocation.processLocation lives in packages/lib/EventManager.ts per repo docs/snippets, not CalEventParser. This will break at build if not re-exported.
Apply this diff if it isn’t re-exported:
-import { processLocation } from "@calcom/lib/CalEventParser"; +import { processLocation } from "@calcom/lib/EventManager";
44-74: Unused param and potential state mismatch (externalCalendarId).build accepts externalCalendarId but never uses it; create uses this.externalCalendarId from the constructor. Either remove this param or persist it to the instance to avoid confusion.
Minimal fix:
async build({ @@ type, }: { @@ type: GoogleCalendarEvent["type"]; }): Promise<GoogleCalendarEvent> { + // Keep instance in sync if caller provided an override + if (externalCalendarId) this.externalCalendarId = externalCalendarId;If you intend constructor-only config, remove externalCalendarId from build and buildAndCreate signatures/usages instead.
348-374: Secrets handling check.serviceAccountKey and credential.key must never be logged or returned. I don’t see exposure here, but please confirm downstream logs also redact keys.
| export interface GoogleCalendarEvent extends CalendarEvent { | ||
| calendarDescription?: string; | ||
| seatsPerTimeSlot?: number; | ||
| seatsShowAttendees?: boolean; | ||
| hideCalendarEventDetails?: boolean; | ||
| recurringEvent?: any; | ||
| existingRecurringEvent?: any; | ||
| iCalUID?: string; | ||
| type: string; | ||
| } |
There was a problem hiding this comment.
Do not redeclare inherited CalendarEvent props (TypeScript conflict).
You’re narrowing types (e.g., iCalUID, seatsPerTimeSlot, seatsShowAttendees) that already exist on CalendarEvent, which will error in TS. Only add the new field.
Apply:
-export interface GoogleCalendarEvent extends CalendarEvent {
- calendarDescription?: string;
- seatsPerTimeSlot?: number;
- seatsShowAttendees?: boolean;
- hideCalendarEventDetails?: boolean;
- recurringEvent?: any;
- existingRecurringEvent?: any;
- iCalUID?: string;
- type: string;
-}
+export interface GoogleCalendarEvent extends CalendarEvent {
+ calendarDescription?: string;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface GoogleCalendarEvent extends CalendarEvent { | |
| calendarDescription?: string; | |
| seatsPerTimeSlot?: number; | |
| seatsShowAttendees?: boolean; | |
| hideCalendarEventDetails?: boolean; | |
| recurringEvent?: any; | |
| existingRecurringEvent?: any; | |
| iCalUID?: string; | |
| type: string; | |
| } | |
| export interface GoogleCalendarEvent extends CalendarEvent { | |
| calendarDescription?: string; | |
| } |
| if (schedulingType === "COLLECTIVE" && teamMembers.length > 0) { | ||
| const teamMemberIds = teamMembers.map((member) => member.id); | ||
|
|
||
| teamDestinationCalendars = await prisma.destinationCalendar.findMany({ | ||
| where: { | ||
| AND: [ | ||
| { | ||
| OR: [{ eventTypeId: booking.eventTypeId }, { userId: { in: teamMemberIds } }], | ||
| }, | ||
| { | ||
| OR: [ | ||
| credentialId ? { credentialId: credentialId } : {}, | ||
| delegationCredentialId ? { delegationCredentialId: delegationCredentialId } : {}, | ||
| ].filter((condition) => Object.keys(condition).length > 0), | ||
| }, | ||
| ], | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prisma OR: [] yields invalid filter when no credential filters are provided.
You always include an OR with a potentially empty array. Prisma rejects OR: [].
Apply:
- teamDestinationCalendars = await prisma.destinationCalendar.findMany({
- where: {
- AND: [
- {
- OR: [{ eventTypeId: booking.eventTypeId }, { userId: { in: teamMemberIds } }],
- },
- {
- OR: [
- credentialId ? { credentialId: credentialId } : {},
- delegationCredentialId ? { delegationCredentialId: delegationCredentialId } : {},
- ].filter((condition) => Object.keys(condition).length > 0),
- },
- ],
- },
- });
+ const credOr = [
+ credentialId ? { credentialId } : null,
+ delegationCredentialId ? { delegationCredentialId } : null,
+ ].filter(Boolean) as Record<string, unknown>[];
+
+ const andFilters: any[] = [
+ { OR: [{ eventTypeId: booking.eventTypeId }, { userId: { in: teamMemberIds } }] },
+ ];
+ if (credOr.length) andFilters.push({ OR: credOr });
+
+ teamDestinationCalendars = await prisma.destinationCalendar.findMany({
+ where: { AND: andFilters },
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (schedulingType === "COLLECTIVE" && teamMembers.length > 0) { | |
| const teamMemberIds = teamMembers.map((member) => member.id); | |
| teamDestinationCalendars = await prisma.destinationCalendar.findMany({ | |
| where: { | |
| AND: [ | |
| { | |
| OR: [{ eventTypeId: booking.eventTypeId }, { userId: { in: teamMemberIds } }], | |
| }, | |
| { | |
| OR: [ | |
| credentialId ? { credentialId: credentialId } : {}, | |
| delegationCredentialId ? { delegationCredentialId: delegationCredentialId } : {}, | |
| ].filter((condition) => Object.keys(condition).length > 0), | |
| }, | |
| ], | |
| }, | |
| }); | |
| } | |
| if (schedulingType === "COLLECTIVE" && teamMembers.length > 0) { | |
| const teamMemberIds = teamMembers.map((member) => member.id); | |
| const credOr = [ | |
| credentialId ? { credentialId } : null, | |
| delegationCredentialId ? { delegationCredentialId } : null, | |
| ].filter(Boolean) as Record<string, unknown>[]; | |
| const andFilters: any[] = [ | |
| { OR: [{ eventTypeId: booking.eventTypeId }, { userId: { in: teamMemberIds } }] }, | |
| ]; | |
| if (credOr.length) andFilters.push({ OR: credOr }); | |
| teamDestinationCalendars = await prisma.destinationCalendar.findMany({ | |
| where: { AND: andFilters }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/features/tasker/calendars/GoogleCalendarProcessor.ts around lines
172 to 190, the current Prisma query always includes an OR with a potentially
empty array (credential filters) which causes Prisma to reject OR: []; fix by
building the credential filter array first and only add the OR clause to the
where when that array is non-empty (or alternatively merge credential filters
into the AND only when present), and ensure you don't pass an empty object into
OR — construct the where object conditionally so OR is omitted if there are no
credential filters.
| const organizerDestinationCalendars = await prisma.destinationCalendar.findMany({ | ||
| where: { | ||
| AND: [ | ||
| { | ||
| OR: [{ eventTypeId: booking.eventTypeId }, { userId: userId }], | ||
| }, | ||
| { | ||
| OR: [ | ||
| credentialId ? { credentialId: credentialId } : {}, | ||
| delegationCredentialId ? { delegationCredentialId: delegationCredentialId } : {}, | ||
| ].filter((condition) => Object.keys(condition).length > 0), | ||
| }, | ||
| ], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same Prisma OR: [] issue for organizerDestinationCalendars.
Apply:
- const organizerDestinationCalendars = await prisma.destinationCalendar.findMany({
- where: {
- AND: [
- {
- OR: [{ eventTypeId: booking.eventTypeId }, { userId: userId }],
- },
- {
- OR: [
- credentialId ? { credentialId: credentialId } : {},
- delegationCredentialId ? { delegationCredentialId: delegationCredentialId } : {},
- ].filter((condition) => Object.keys(condition).length > 0),
- },
- ],
- },
- });
+ {
+ const credOr = [
+ credentialId ? { credentialId } : null,
+ delegationCredentialId ? { delegationCredentialId } : null,
+ ].filter(Boolean) as Record<string, unknown>[];
+ const andFilters: any[] = [{ OR: [{ eventTypeId: booking.eventTypeId }, { userId }] }];
+ if (credOr.length) andFilters.push({ OR: credOr });
+ var organizerDestinationCalendars = await prisma.destinationCalendar.findMany({
+ where: { AND: andFilters },
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const organizerDestinationCalendars = await prisma.destinationCalendar.findMany({ | |
| where: { | |
| AND: [ | |
| { | |
| OR: [{ eventTypeId: booking.eventTypeId }, { userId: userId }], | |
| }, | |
| { | |
| OR: [ | |
| credentialId ? { credentialId: credentialId } : {}, | |
| delegationCredentialId ? { delegationCredentialId: delegationCredentialId } : {}, | |
| ].filter((condition) => Object.keys(condition).length > 0), | |
| }, | |
| ], | |
| }, | |
| }); | |
| { | |
| const credOr = [ | |
| credentialId ? { credentialId } : null, | |
| delegationCredentialId ? { delegationCredentialId } : null, | |
| ].filter(Boolean) as Record<string, unknown>[]; | |
| const andFilters: any[] = [ | |
| { OR: [{ eventTypeId: booking.eventTypeId }, { userId }] }, | |
| ]; | |
| if (credOr.length) { | |
| andFilters.push({ OR: credOr }); | |
| } | |
| var organizerDestinationCalendars = await prisma.destinationCalendar.findMany({ | |
| where: { AND: andFilters }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/features/tasker/calendars/GoogleCalendarProcessor.ts around lines
231 to 245, the Prisma query can produce an empty OR: [] for the
credential/delegation clause (and potentially for the top-level OR) which Prisma
rejects; instead build the where object conditionally: construct arrays for each
OR (e.g., credentialConditions and topLevelConditions), push candidate objects
only when their values exist, and only include an OR property in the final where
if the corresponding array.length > 0 (omit it otherwise) so the query never
contains an empty array.
| const bookerUrl = await getBookerBaseUrl(teamParentId ?? organizerOrganizationId ?? null); | ||
|
|
||
| const eventNameObject = { | ||
| attendeeName: booking.attendees[0]?.name || "Nameless", | ||
| eventType: type, | ||
| eventName: booking.title || type, | ||
| teamName: schedulingType === "COLLECTIVE" ? teamObject?.name : null, | ||
| host: organizerUser.name || "Nameless", | ||
| location: booking.location, | ||
| eventDuration: dayjs(booking.endTime).diff(booking.startTime, "minutes"), | ||
| bookingFields: {}, | ||
| t: organizerTranslation, | ||
| }; | ||
| const eventName = getEventName(eventNameObject); | ||
|
|
There was a problem hiding this comment.
bookerUrl computed but never set on event.
getRichDescription may rely on event.bookerUrl. Set it before composing calendarDescription.
Apply:
const eventName = getEventName(eventNameObject);
@@
calendarEvent = {
...processLocation(calendarEvent),
};
+
+ // Ensure manage links and descriptions can include booker URL
+ calendarEvent.bookerUrl = bookerUrl ?? undefined;
@@
calendarEvent.calendarDescription = getRichDescription(calendarEvent);Also applies to: 332-341
🤖 Prompt for AI Agents
In packages/features/tasker/calendars/GoogleCalendarProcessor.ts around lines
290 to 304, the computed bookerUrl is never assigned to the event object before
composing the calendar description; set the computed value onto the event (e.g.,
event.bookerUrl = bookerUrl) immediately after computing bookerUrl and before
calling getRichDescription/getEventName so that downstream description builders
can read it; apply the same assignment in the corresponding block around lines
332 to 341.
| const regularCredential = await prisma.credential.findUnique({ | ||
| where: { id: this.credentialId }, | ||
| include: { | ||
| user: { | ||
| select: { | ||
| email: true, | ||
| }, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prisma guideline: avoid include; use select.
Replace include with select to fetch only needed fields.
Apply:
- const regularCredential = await prisma.credential.findUnique({
- where: { id: this.credentialId },
- include: {
- user: {
- select: {
- email: true,
- },
- },
- },
- });
+ const regularCredential = await prisma.credential.findUnique({
+ where: { id: this.credentialId },
+ select: {
+ id: true,
+ type: true,
+ key: true,
+ userId: true,
+ appId: true,
+ invalid: true,
+ user: { select: { email: true } },
+ },
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const regularCredential = await prisma.credential.findUnique({ | |
| where: { id: this.credentialId }, | |
| include: { | |
| user: { | |
| select: { | |
| email: true, | |
| }, | |
| }, | |
| }, | |
| }); | |
| const regularCredential = await prisma.credential.findUnique({ | |
| where: { id: this.credentialId }, | |
| select: { | |
| id: true, | |
| type: true, | |
| key: true, | |
| userId: true, | |
| appId: true, | |
| invalid: true, | |
| user: { | |
| select: { | |
| email: true, | |
| }, | |
| }, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In packages/features/tasker/calendars/GoogleCalendarProcessor.ts around lines
375 to 384, replace the prisma.findUnique call that uses include with a select
projection to fetch only the required fields (credential fields plus nested
user.email) to follow the Prisma guideline; update the query to use select with
a nested user: { select: { email: true } } and adjust any consuming code/type
expectations accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
turbo.json (1)
25-25: Remove duplicate entries from turbo.json’s globalEnv
- Detected duplicates in the globalEnv array:
• CALCOM_CREDENTIAL_SYNC_SECRET
• INTERCOM_SECRET
• NEXT_PUBLIC_ORG_SELF_SERVE_ENABLED
• VAPID_PRIVATE_KEY- Remove the redundant occurrences to slim the cache key and eliminate noise.
📜 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)
turbo.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
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.
⏰ 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 (1)
turbo.json (1)
260-262: ScopeTRIGGER_SECRET_KEYandTRIGGER_API_URLto only tasks that consume them.
These keys currently live inglobalEnv(turbo.json lines 261–262), which causes any change to invalidate every task’s cache. Move them into theenvarray of only those tasks that actually referenceprocess.env.TRIGGER_SECRET_KEYorprocess.env.TRIGGER_API_URL. I ran a repo-wide grep and found no code references outside turbo.json—please locate the real consumers and adjust accordingly.
| "BUILD_STANDALONE", | ||
| "TRIGGER_SECRET_KEY", | ||
| "TRIGGER_API_URL" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure env documentation and CI secrets are updated.
Add TRIGGER_SECRET_KEY and TRIGGER_API_URL to .env.example/.env.appStore.example, mask them in CI logs, and provision them in all deployment environments.
Suggested checks:
🏁 Script executed:
#!/bin/bash
# Verify examples include new vars
rg -n 'TRIGGER_SECRET_KEY|TRIGGER_API_URL' .env.example .env.appStore.example || echo "Missing in env examples"
# Spot references in CI configs (adjust paths if different)
fd -a -HI -t f -E node_modules -E .git -g '(*ci*|*.yml|*.yaml)' | xargs rg -n 'TRIGGER_SECRET_KEY|TRIGGER_API_URL' -n || echo "No CI references found (may be fine)"Length of output: 334
Add new env vars to examples & CI
- Insert
TRIGGER_SECRET_KEYandTRIGGER_API_URLinto.env.exampleand.env.appStore.example(currently missing). - Update your CI/YAML configs to mask these variables and reference them where needed.
- Ensure both keys are provisioned in every deployment environment.
🤖 Prompt for AI Agents
In turbo.json around lines 260 to 262, the review requests adding the new
environment variables TRIGGER_SECRET_KEY and TRIGGER_API_URL into the example
env files and CI: update .env.example and .env.appStore.example to include these
two keys (with placeholder values or comments), update CI/YAML pipeline configs
to add and mask TRIGGER_SECRET_KEY and TRIGGER_API_URL as secret variables and
reference them in any steps that need triggering, and ensure deployment
manifests or environment provisioning scripts include these variables so every
environment (dev/staging/prod) has them set.
|
This PR is being marked as stale due to inactivity. |
|
closing this PR, making a new one |
3 participants
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist