feat: cal ai self serve architecture #1 #22919
Conversation
WalkthroughThis update introduces a comprehensive AI Phone Service architecture centered around a new, extensible provider abstraction, with an initial implementation for Retell AI. The changes include new TypeScript interfaces defining provider contracts, a provider registry and factory pattern, Prisma-backed repositories, service classes for configuration, agent, phone number, billing, and call management, and extensive test suites for each component. The Prisma schema and migrations are updated to support new Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/06/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add foundation team as reviewer" took an action on this PR • (08/06/25)1 reviewer was added to this PR based on Keith Williams's automation. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
There was a problem hiding this comment.
Actionable comments posted: 18
🔭 Outside diff range comments (1)
packages/features/ee/cal-ai-phone/README.md (1)
1-765: Consider splitting this PR into smaller, focused pull requestsThis PR introduces a comprehensive AI phone service feature with extensive changes across multiple layers (database, repositories, services, providers, UI components). Based on best practices, large PRs (>500 lines or >10 files) should be split into smaller, manageable pieces for easier review and testing.
Suggested split:
- Database & Core Models: Migration, Prisma schema, and repository layer
- Provider Abstraction: Interfaces and registry system
- Retell AI Implementation: Provider, service, client, and mappers
- UI Components: Agent and phone number management pages
- Workflow Integration: Workflow step configuration and execution
- Documentation & Tests: README and test suites
This approach would improve reviewability, reduce merge conflicts, and allow for incremental deployment.
🧹 Nitpick comments (16)
packages/features/ee/cal-ai-phone/providers/retell-ai/factory.test.ts (2)
76-76: Improve type safety in test assertion.The use of
anytype assertion weakens type safety. Consider using a more specific type or a helper function.- const mockLoggerInstance = (logger.getSubLogger as any).mock.results[0].value; + const mockLoggerInstance = vi.mocked(logger.getSubLogger).mock.results[0]?.value;
118-118: Improve type safety in test assertion.Similar to the previous comment, avoid
anytype assertion for better type safety.- const clientInstance = (RetellSDKClient as any).mock.results[0].value; + const clientInstance = vi.mocked(RetellSDKClient).mock.results[0]?.value;packages/features/ee/cal-ai-phone/providers/retell-ai/client.test.ts (1)
117-132: Effective API key validation testing, but consider simplifying async usage.The test correctly validates the missing API key scenario using dynamic imports and module resets. However, the test function is marked async but the constructor call itself is synchronous - the async is only needed for the dynamic import.
packages/features/ee/cal-ai-phone/providers/retell-ai/factory.ts (1)
11-32: Factory pattern implementation is correct with good logging flexibility.The factory correctly implements the required interface and provides flexible logging configuration. Both methods properly instantiate the SDK client and provider.
Consider consolidating the duplicate logging logic between the two methods to reduce code duplication:
+ private static createLogger(config?: { enableLogging?: boolean; logger?: ReturnType<typeof logger.getSubLogger> }): ReturnType<typeof logger.getSubLogger> | undefined { + const enableLogging = config?.enableLogging ?? true; + return enableLogging + ? config?.logger || logger.getSubLogger({ prefix: ["retellAIProvider:"] }) + : undefined; + } create(config: AIPhoneServiceProviderConfig): AIPhoneServiceProvider { - const log = - config.enableLogging !== false ? logger.getSubLogger({ prefix: ["retellAIProvider:"] }) : undefined; + const log = RetellAIProviderFactory.createLogger({ enableLogging: config.enableLogging !== false }); const sdkClient = new RetellSDKClient(log); return new RetellAIProvider(sdkClient); } static createWithConfig(config?: { enableLogging?: boolean; logger?: ReturnType<typeof logger.getSubLogger>; }): AIPhoneServiceProvider { - const enableLogging = config?.enableLogging ?? true; - const log = enableLogging - ? config?.logger || logger.getSubLogger({ prefix: ["retellAIProvider:"] }) - : undefined; + const log = RetellAIProviderFactory.createLogger(config); const sdkClient = new RetellSDKClient(log); return new RetellAIProvider(sdkClient); }packages/features/ee/cal-ai-phone/providers/retell-ai/provider.test.ts (1)
17-43: Consider using a type-safe mocking approachThe type casting
as unknown as RetellAIServicebypasses TypeScript's type safety. If theRetellAIServiceinterface changes, this mock might not reflect those changes, leading to false-positive tests.Consider using a factory function that implements the full interface or a mocking library that provides type-safe mocks:
-function createMockRetellAIService(overrides: Partial<RetellAIService> = {}): RetellAIService { - const defaultMocks = { - setupAIConfiguration: vi.fn(), - // ... other mocks - }; - - return { ...defaultMocks, ...overrides } as unknown as RetellAIService; -} +function createMockRetellAIService(overrides: Partial<RetellAIService> = {}): RetellAIService { + const defaultMocks: RetellAIService = { + setupAIConfiguration: vi.fn(), + deleteAIConfiguration: vi.fn(), + updateLLMConfiguration: vi.fn(), + getLLMDetails: vi.fn(), + getAgent: vi.fn(), + updateAgent: vi.fn(), + createPhoneCall: vi.fn(), + createPhoneNumber: vi.fn(), + importPhoneNumber: vi.fn(), + deletePhoneNumber: vi.fn(), + getPhoneNumber: vi.fn(), + updatePhoneNumber: vi.fn(), + generatePhoneNumberCheckoutSession: vi.fn(), + cancelPhoneNumberSubscription: vi.fn(), + updatePhoneNumberWithAgents: vi.fn(), + listAgents: vi.fn(), + getAgentWithDetails: vi.fn(), + createAgent: vi.fn(), + updateAgentConfiguration: vi.fn(), + deleteAgent: vi.fn(), + createTestCall: vi.fn(), + }; + + return { ...defaultMocks, ...overrides }; +}packages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.ts (1)
81-91: Consider using a more robust unique identifierWhile
Date.now()provides uniqueness in most cases, consider using a UUID or nanoid for guaranteed uniqueness, especially in high-concurrency scenarios.-agent_name: `agent-${eventTypeId || "default"}-${Date.now()}`, +agent_name: `agent-${eventTypeId || "default"}-${crypto.randomUUID()}`,packages/features/ee/cal-ai-phone/README.md (5)
11-31: Add language specifier to architecture diagramAdd a language specifier to the fenced code block for better markdown rendering.
-``` +```text ┌─────────────────────────────────────────────┐
318-328: Add language specifier to workflow architecture diagram-``` +```text ┌─────────────────────────────────────────────┐
416-428: Add language specifier to execution flow diagram-``` +```text 1. Workflow Trigger (e.g., 24 hours before event)
446-460: Add language specifier to data flow diagram-``` +```text 1. Workflow Trigger Event
679-695: Add language specifier to credit deduction flow diagram-``` +```text 1. AI Phone Call Completespackages/features/ee/cal-ai-phone/providers/retell-ai/service.ts (3)
133-139: Consider bubbling up cleanup failures to the userWhen cleanup of the imported phone number fails (line 137), the error is only logged. This could leave orphaned resources in Retell that the user is unaware of.
Consider adding a more descriptive error message that indicates partial failure:
if (importedPhoneNumber?.phone_number) { try { await this.repository.deletePhoneNumber(importedPhoneNumber.phone_number); } catch (cleanupError) { console.error("Failed to cleanup Retell phone number:", cleanupError); + // Append cleanup failure info to the error message + const originalError = error instanceof Error ? error.message : "Failed to import phone number"; + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: `${originalError}. Additionally, cleanup of partially imported resources failed. Please contact support.`, + }); } }
361-361: Extract hardcoded API path to a constantThe success URL path is hardcoded inline, which could lead to maintenance issues if the API structure changes.
Consider extracting to a constant at the top of the file:
const PHONE_NUMBER_SUBSCRIPTION_SUCCESS_PATH = "/api/phone-numbers/subscription/success";Then use it in the checkout session:
- success_url: `${WEBAPP_URL}/api/phone-numbers/subscription/success?session_id={CHECKOUT_SESSION_ID}`, + success_url: `${WEBAPP_URL}${PHONE_NUMBER_SUBSCRIPTION_SUCCESS_PATH}?session_id={CHECKOUT_SESSION_ID}`,
643-643: Consider using a more robust ID generation methodUsing
Math.random()for generating unique identifiers could potentially lead to collisions.Consider using a UUID library or timestamp-based approach:
- const agentName = _name || `Agent - ${userId} ${Math.random().toString(36).substring(2, 15)}`; + const agentName = _name || `Agent - ${userId} ${Date.now().toString(36)}`;Or better yet, use a proper UUID library if available in the project.
packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts (1)
687-860: Exceptional test coverage for createTestCall with all edge casesExcellent coverage of credit validation, rate limiting, and error scenarios. The tests properly handle edge cases like null/undefined credits and validate all preconditions.
Consider adding a test case that validates the passing of dynamic variables to the phone call creation, similar to the createPhoneCall test at line 381-404.
packages/features/ee/cal-ai-phone/providers/retell-ai/client.ts (1)
34-48: Good error handling and logging for LLM operationsThe implementation follows consistent patterns for error handling and logging.
Consider using optional chaining for safer event_type_id extraction at line 36:
- eventTypeId: data.general_tools?.find((t) => "event_type_id" in t)?.event_type_id, + eventTypeId: data.general_tools?.find((t) => t && typeof t === 'object' && "event_type_id" in t)?.event_type_id,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (25)
apps/web/package.json(2 hunks)packages/features/ee/cal-ai-phone/README.md(1 hunks)packages/features/ee/cal-ai-phone/ai-phone-service-registry.test.ts(1 hunks)packages/features/ee/cal-ai-phone/ai-phone-service-registry.ts(1 hunks)packages/features/ee/cal-ai-phone/index.ts(1 hunks)packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/client.test.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/client.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/errors.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/factory.test.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/factory.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/index.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/provider.test.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/types.ts(1 hunks)packages/features/ee/cal-ai-phone/template-fields-map.ts(1 hunks)packages/features/ee/cal-ai-phone/zod-utils.ts(2 hunks)packages/lib/constants.ts(1 hunks)packages/lib/server/repository/agent.ts(1 hunks)packages/lib/server/repository/phoneNumber.ts(1 hunks)packages/prisma/migrations/20250806084401_add_cal_ai_self_serve/migration.sql(1 hunks)packages/prisma/schema.prisma(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/ee/cal-ai-phone/template-fields-map.tspackages/features/ee/cal-ai-phone/providers/retell-ai/index.tspackages/features/ee/cal-ai-phone/providers/retell-ai/errors.tspackages/features/ee/cal-ai-phone/providers/retell-ai/factory.test.tspackages/lib/constants.tspackages/features/ee/cal-ai-phone/providers/retell-ai/factory.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/client.test.tspackages/lib/server/repository/agent.tspackages/features/ee/cal-ai-phone/zod-utils.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.test.tspackages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/client.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/lib/server/repository/phoneNumber.tspackages/features/ee/cal-ai-phone/index.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.tspackages/features/ee/cal-ai-phone/providers/retell-ai/types.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.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/ee/cal-ai-phone/template-fields-map.tspackages/features/ee/cal-ai-phone/providers/retell-ai/index.tspackages/features/ee/cal-ai-phone/providers/retell-ai/errors.tspackages/features/ee/cal-ai-phone/providers/retell-ai/factory.test.tspackages/lib/constants.tspackages/features/ee/cal-ai-phone/providers/retell-ai/factory.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/client.test.tspackages/lib/server/repository/agent.tspackages/features/ee/cal-ai-phone/zod-utils.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.test.tspackages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/client.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/lib/server/repository/phoneNumber.tspackages/features/ee/cal-ai-phone/index.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.tspackages/features/ee/cal-ai-phone/providers/retell-ai/types.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
📚 Learning: in the cal.com cal ai phone feature, the deletephonenumber repository method validation is properly ...
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/index.tspackages/features/ee/cal-ai-phone/providers/retell-ai/factory.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/client.test.tspackages/lib/server/repository/agent.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.test.tspackages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/client.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/prisma/migrations/20250806084401_add_cal_ai_self_serve/migration.sqlpackages/features/ee/cal-ai-phone/README.mdpackages/lib/server/repository/phoneNumber.tspackages/features/ee/cal-ai-phone/index.tspackages/features/ee/cal-ai-phone/providers/retell-ai/types.tspackages/prisma/schema.prismapackages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts
📚 Learning: in packages/app-store/office365calendar/lib/calendarservice.ts, the fetcher method in office365calen...
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/ee/cal-ai-phone/providers/retell-ai/factory.test.tspackages/features/ee/cal-ai-phone/README.md
📚 Learning: applies to **/*.ts : ensure the `credential.key` field is never returned from trpc endpoints or apis...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : Ensure the `credential.key` field is never returned from tRPC endpoints or APIs
Applied to files:
packages/lib/constants.ts
📚 Learning: cal.com integrations: `webapp_url_for_oauth` intentionally falls back to `http://localhost:3000` whe...
Learnt from: hariombalhara
PR: calcom/cal.com#22840
File: packages/app-store/webex/api/add.ts:4-4
Timestamp: 2025-08-04T13:14:39.218Z
Learning: Cal.com integrations: `WEBAPP_URL_FOR_OAUTH` intentionally falls back to `http://localhost:3000` when neither production nor development, matching the pattern used by other apps in the repo.
Applied to files:
packages/lib/constants.ts
📚 Learning: applies to **/*repository.ts : repository files must include `repository` suffix, prefix with techno...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Repository.ts : Repository files must include `Repository` suffix, prefix with technology if applicable (e.g., `PrismaAppRepository.ts`), and use PascalCase matching the exported class
Applied to files:
packages/lib/server/repository/agent.tspackages/lib/server/repository/phoneNumber.ts
📚 Learning: applies to **/*service.ts : service files must include `service` suffix, use pascalcase matching exp...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Service.ts : Service files must include `Service` suffix, use PascalCase matching exported class, and avoid generic names (e.g., `MembershipService.ts`)
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
🧬 Code Graph Analysis (6)
packages/features/ee/cal-ai-phone/template-fields-map.ts (2)
packages/features/ee/cal-ai-phone/index.ts (1)
templateFieldsMap(50-50)packages/features/ee/cal-ai-phone/zod-utils.ts (2)
TemplateType(80-80)Fields(121-121)
packages/features/ee/cal-ai-phone/providers/retell-ai/errors.ts (2)
packages/features/ee/cal-ai-phone/index.ts (1)
RetellAIError(31-31)packages/features/ee/cal-ai-phone/providers/retell-ai/index.ts (1)
RetellAIError(5-5)
packages/features/ee/cal-ai-phone/providers/retell-ai/factory.test.ts (4)
packages/features/ee/cal-ai-phone/providers/retell-ai/factory.ts (1)
RetellAIProviderFactory(11-32)packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts (1)
AIPhoneServiceProviderConfig(258-263)packages/features/ee/cal-ai-phone/providers/retell-ai/client.ts (1)
RetellSDKClient(18-235)packages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts (1)
RetellAIProvider(21-233)
packages/lib/server/repository/agent.ts (1)
packages/platform/libraries/index.ts (1)
MembershipRole(98-98)
packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts (2)
packages/features/ee/cal-ai-phone/index.ts (12)
AIPhoneServiceConfiguration(6-6)AIPhoneServiceModel(10-10)AIPhoneServiceDeletion(7-7)AIPhoneServiceDeletionResult(8-8)AIPhoneServiceCallData(9-9)AIPhoneServiceAgent(11-11)AIPhoneServiceCall(12-12)AIPhoneServicePhoneNumber(13-13)AIPhoneServiceProvider(3-3)AIPhoneServiceProviderConfig(5-5)AIPhoneServiceProviderFactory(4-4)AIPhoneServiceProviderType(16-16)packages/features/ee/cal-ai-phone/providers/retell-ai/types.ts (1)
RetellAgentWithDetails(93-116)
packages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts (3)
packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts (8)
AIPhoneServiceProvider(80-253)AIPhoneServiceConfiguration(17-17)AIPhoneServiceDeletion(21-24)AIPhoneServiceModel(19-19)AIPhoneServiceAgent(36-36)AIPhoneServiceCallData(35-35)AIPhoneServiceCall(37-37)AIPhoneServicePhoneNumber(39-39)packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts (1)
RetellAIService(34-857)packages/features/ee/cal-ai-phone/providers/retell-ai/types.ts (2)
RetellAIRepository(141-163)RetellAgentWithDetails(93-116)
🪛 markdownlint-cli2 (0.17.2)
packages/features/ee/cal-ai-phone/README.md
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
318-318: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
416-416: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
446-446: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
679-679: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. 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 (46)
apps/web/package.json (1)
3-3: Question: Why was the version downgraded?The version was downgraded from 5.5.13 to 5.5.11. This is unusual unless there was a rollback for a specific reason. Please clarify the rationale for the version downgrade.
packages/lib/constants.ts (1)
24-27: RETELL_API_KEY usage verified server-only
All references to RETELL_API_KEY are confined to server-side modules (packages/lib/constants.ts and packages/features/ee/cal-ai-phone/*). No imports were found in any web-client or UI code.packages/features/ee/cal-ai-phone/template-fields-map.ts (1)
4-4: LGTM! Improved naming convention.Renaming from
TEMPLATES_FIELDStotemplateFieldsMapimproves consistency with TypeScript/JavaScript naming conventions while preserving all functionality.packages/features/ee/cal-ai-phone/providers/retell-ai/errors.ts (1)
1-6: LGTM! Well-implemented error class.The
RetellAIErrorclass follows best practices:
- Properly extends native Error
- Sets the name property correctly
- Includes contextual information (operation)
- Supports error chaining with originalError
- Clean, focused implementation
packages/features/ee/cal-ai-phone/providers/retell-ai/factory.test.ts (1)
1-122: Overall test implementation looks good.The test suite provides comprehensive coverage of the factory behavior:
- Proper mocking strategy for all dependencies
- Tests both logging enabled/disabled scenarios
- Verifies dependency injection patterns
- Clear test structure and assertions
- Good coverage of static and instance methods
packages/features/ee/cal-ai-phone/providers/retell-ai/client.test.ts (5)
1-58: Comprehensive mocking setup follows testing best practices.The mocking configuration properly isolates external dependencies and provides a complete interface that mirrors the actual SDK structure. The logger and constants mocking ensures tests run independently of external configuration.
135-198: Excellent CRUD test coverage for LLM operations.The test suite comprehensively covers all LLM operations with proper mock verification and parameter validation. The consistent test structure makes it easy to understand and maintain.
200-265: Consistent and comprehensive Agent operations testing.The Agent operation tests maintain the same high quality and structure as the LLM tests, providing complete CRUD coverage with proper mock verification.
267-350: Comprehensive phone number operations test coverage.The tests cover all phone number operations including the specialized import functionality, with realistic test data and proper parameter validation for complex request objects.
352-411: Good coverage of call operations and practical edge cases.The tests handle both normal and edge case scenarios effectively, including undefined dynamic variables and special characters in phone numbers, which are common real-world scenarios.
packages/features/ee/cal-ai-phone/providers/retell-ai/index.ts (1)
1-99: Excellent module organization with comprehensive usage documentation.The exports are well-structured and the extensive usage examples provide clear guidance for both the new provider abstraction pattern and legacy service usage. The examples include practical scenarios like error handling and configuration management.
packages/features/ee/cal-ai-phone/ai-phone-service-registry.test.ts (3)
1-74: Comprehensive test setup with proper mocking and state management.The mocking setup covers all dependencies and provides a complete mock provider implementation. The beforeEach hooks ensure proper test isolation by clearing registry state.
75-204: Comprehensive registry operation testing with good error coverage.The tests thoroughly cover all registry operations including provider registration, factory retrieval, provider creation, and default provider management. Error scenarios and state management are properly tested.
206-359: Thorough testing of helper functions with good configuration coverage.The tests comprehensively validate the helper functions' behavior including configuration merging, default value handling, and parameter overrides. The test coverage ensures reliable operation of the convenience functions.
packages/features/ee/cal-ai-phone/zod-utils.ts (2)
47-47: Logical addition of required phone_number field to the schema.Adding the
phone_numberfield to the get phone number response schema is appropriate as the response should include the queried phone number. The field naming follows API conventions.
177-200: Well-designed response schemas with comprehensive validation.The new response schemas provide thorough validation coverage with appropriate field types, optional/nullable handling, and URL validation. The schema reuse pattern for update responses is efficient and maintains consistency.
packages/lib/server/repository/phoneNumber.ts (1)
153-159: Incorrect security concern: repository deletePhoneNumber is safe as-isThe
userIdparameter is declared but never used, and ownership is already enforced in the service layer (defense-in-depth at the ORM level isn’t required becausephoneNumberis a unique identifier). Rather than adding a redundant filter, it’s clearer to remove the unused parameter from the repository API:• Remove the
userIdparam from the signature and its type.
• Keep deleting by the uniquephoneNumberonly.- static async deletePhoneNumber({ phoneNumber }: { phoneNumber: string; userId: number }) { + static async deletePhoneNumber(phoneNumber: string) { return await prisma.calAiPhoneNumber.delete({ where: { phoneNumber }, }); }Likely an incorrect or invalid review comment.
packages/lib/server/repository/agent.ts (5)
33-41: Secure implementation for team access retrievalThe method correctly uses parameterized queries to prevent SQL injection and properly filters for accepted team memberships.
43-53: Well-implemented admin access checkThe method correctly validates admin privileges by checking for both ADMIN and OWNER roles, using proper parameterization and enum values from the MembershipRole enum.
192-206: Excellent N+1 query preventionGreat optimization by fetching phone numbers in a separate bulk query rather than loading them individually for each agent. This significantly improves performance when dealing with multiple agents.
359-378: Clean create implementationSimple and effective use of Prisma ORM for agent creation.
456-473: Proper permission validation for team managementThe method correctly validates that users have ADMIN or OWNER roles to manage team resources, with appropriate bigint handling for the COUNT result.
packages/features/ee/cal-ai-phone/index.ts (1)
1-93: Well-organized module exports with helpful documentationThe index file provides a clean, centralized export structure with comprehensive usage examples. Good practice maintaining backward compatibility with legacy exports.
packages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.ts (1)
27-62: Correct tool configuration builderThe method properly constructs the tools array with required end_call tool and optional calendar tools based on configuration.
packages/prisma/schema.prisma (6)
1326-1326: LGTM!The new workflow action follows the existing naming convention.
1345-1346: LGTM!Proper optional relation setup with appropriate cascade behavior.
1484-1484: LGTM!The new workflow method follows the existing naming convention.
2424-2432: LGTM!Comprehensive subscription status enum covering all typical states.
2435-2459: LGTM!Well-structured Agent model with proper indexes, constraints, and cascade behavior.
2461-2494: LGTM!Well-designed phone number model with appropriate constraints, indexes, and relationship handling.
packages/features/ee/cal-ai-phone/providers/retell-ai/types.ts (3)
1-42: LGTM!Well-structured type definitions with proper type guards for runtime safety.
44-80: LGTM!Comprehensive language support with standard locale codes.
141-163: LGTM!Clean repository interface with well-defined async operations and clear separation of concerns.
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts (2)
37-54: LGTM!The method properly sets up AI configuration with appropriate defaults and follows a logical flow.
256-309: Well-structured deletion with proper validationThe method correctly validates ownership and subscription status before deletion, which aligns with the proper service layer validation pattern. The graceful handling of agent removal failures is appropriate.
packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts (7)
1-76: Well-structured test setup with comprehensive mockingThe mock setup properly covers all dependencies including repositories, Stripe integration, credit services, and Prisma transactions. Good practice using vi.mock for module-level mocking.
77-122: Clean test isolation with proper setup and teardownGood practice with beforeEach/afterEach hooks ensuring clean test isolation. The transaction mock implementation provides flexibility for testing both success and failure scenarios.
123-168: Comprehensive coverage of AI configuration setup scenariosTests properly verify both minimal configuration and enhanced configuration with Cal.com tools. Good use of expect.objectContaining for flexible assertion matching.
170-266: Thorough testing of deletion operations with proper error handlingExcellent coverage of edge cases including 404 errors, partial failures, and business rule validation for phone number subscription statuses. The tests properly validate that the service layer handles authorization as mentioned in the retrieved learnings.
268-379: Excellent transaction and error handling test coverageGreat testing of transactional behavior including rollback scenarios and cleanup on failure. The agent permission validation test ensures proper authorization checks. The transaction rollback test with phone number cleanup is particularly valuable for ensuring data consistency.
381-638: Comprehensive CRUD operation test coverageAll service methods are properly tested with appropriate assertions. Good coverage of Stripe integration including error cases for missing price IDs. The agent listing test properly verifies user access control.
639-685: Well-tested agent lifecycle operationsGood coverage of agent creation and deletion including permission checks and resource cleanup.
packages/features/ee/cal-ai-phone/providers/retell-ai/client.ts (4)
1-33: Well-structured client initialization with proper API key validationGood practice validating the API key at construction time and providing clear error messages. The customizable logger is a nice touch for debugging.
89-150: Consistent and well-structured agent operationsAll agent methods follow the same robust pattern with proper logging context and error handling.
152-216: Well-implemented phone number operations with proper null handlingGood separation between create and import operations. The update method properly handles nullable agent IDs.
218-235: Clean implementation of phone call creationGood support for optional dynamic variables and consistent error handling pattern.
packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts
Outdated
Show resolved
Hide resolved
packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts
Outdated
Show resolved
Hide resolved
packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts
Outdated
Show resolved
Hide resolved
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
Outdated
Show resolved
Hide resolved
packages/prisma/migrations/20250806084401_add_cal_ai_self_serve/migration.sql
Outdated
Show resolved
Hide resolved
packages/prisma/migrations/20250806084401_add_cal_ai_self_serve/migration.sql
Outdated
Show resolved
Hide resolved
packages/prisma/migrations/20250806084401_add_cal_ai_self_serve/migration.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (12)
packages/features/ee/cal-ai-phone/zod-utils.ts (1)
47-47: Add phone number validation for consistency.The
phone_numberfield inZGetPhoneNumberSchemalacks validation, while other phone number fields in the file useisValidPhoneNumbervalidation. Consider adding the same validation for consistency.- phone_number: z.string(), + phone_number: z.string().refine((val) => isValidPhoneNumber(val)),packages/features/ee/cal-ai-phone/ai-phone-service-registry.test.ts (1)
170-172: Avoid accessing private properties in tests.Using type assertion to access private property
defaultProviderbreaks encapsulation. Consider adding a public method to reset the default provider for testing purposes.Instead of:
(AIPhoneServiceRegistry as any).defaultProvider = AIPhoneServiceProviderType.RETELL_AI;Add a test utility method to the registry:
// In AIPhoneServiceRegistry class static resetToDefault(): void { this.defaultProvider = AIPhoneServiceProviderType.RETELL_AI; }packages/features/ee/cal-ai-phone/ai-phone-service-registry.ts (1)
73-73: Consider lazy initialization for provider registration.Registering the provider at module load time could cause initialization order issues if RetellAIProviderFactory has dependencies that aren't ready. Consider lazy initialization or an explicit initialization method.
Instead of static initialization:
-AIPhoneServiceRegistry.registerProvider(AIPhoneServiceProviderType.RETELL_AI, new RetellAIProviderFactory()); +// Initialize default providers lazily +export function initializeDefaultProviders(): void { + if (!AIPhoneServiceRegistry.isProviderRegistered(AIPhoneServiceProviderType.RETELL_AI)) { + AIPhoneServiceRegistry.registerProvider(AIPhoneServiceProviderType.RETELL_AI, new RetellAIProviderFactory()); + } +}packages/lib/server/repository/agent.ts (1)
6-31: Consider TypeScript naming conventions for internal interfaces.The underscore prefix for private interfaces is unconventional in TypeScript. Consider using a more standard approach.
-interface _AgentRawResult { +interface AgentRawResult {Or move these to a separate non-exported namespace:
namespace Internal { export interface AgentRawResult { // ... } }packages/features/ee/cal-ai-phone/README.md (1)
11-31: Add language specifications to fenced code blocks.The markdown linter correctly identifies that these code blocks lack language specifications. For ASCII diagrams and flow charts, use
textor consider usingmermaidfor better visualization.For ASCII diagrams, add
textlanguage specification:-``` +```text ┌─────────────────────────────────────────────┐ │ Frontend (React/Next.js) │ ...Or consider using Mermaid for better visualization:
-``` +```mermaid graph TD A[Frontend React/Next.js] --> B[TRPC API Layer] B --> C[Service Layer] C --> D[Repository Layer] D --> E[Prisma ORM/Database]Also applies to: 318-328, 416-428, 446-460, 679-695
packages/features/ee/cal-ai-phone/providers/retell-ai/client.ts (2)
63-75: Move initial logging outside the try block.The initial logging statement should be outside the try block to avoid catching logger errors along with SDK errors.
async updateLLM(llmId: string, data: UpdateLLMRequest) { + this.logger.info("Updating LLM via SDK", { llmId }); + try { - this.logger.info("Updating LLM via SDK", { llmId }); - const response = await this.client.llm.update(llmId, data); this.logger.info("LLM updated successfully", { llmId }); return response; } catch (error) { this.logger.error("Failed to update LLM", { error, llmId }); throw error; } }
152-181: Add missing logging statements for consistency.The
createPhoneNumberandimportPhoneNumbermethods are missing initial info logs that other methods have. This inconsistency makes debugging harder.async createPhoneNumber(data: CreatePhoneNumberParams) { + this.logger.info("Creating phone number via SDK", { + area_code: data.area_code, + nickname: data.nickname, + }); + try { const response = await this.client.phoneNumber.create({ area_code: data.area_code, inbound_agent_id: data.inbound_agent_id, outbound_agent_id: data.outbound_agent_id, nickname: data.nickname, }); + this.logger.info("Phone number created successfully", { + phoneNumber: response.phone_number, + }); return response; } catch (error) { this.logger.error("Failed to create phone number", { error }); throw error; } } async importPhoneNumber(data: ImportPhoneNumberParams) { + this.logger.info("Importing phone number via SDK", { + phone_number: data.phone_number, + nickname: data.nickname, + }); + try { const response = await this.client.phoneNumber.import({ phone_number: data.phone_number, termination_uri: data.termination_uri, sip_trunk_auth_username: data.sip_trunk_auth_username, sip_trunk_auth_password: data.sip_trunk_auth_password, nickname: data.nickname, }); + this.logger.info("Phone number imported successfully", { + phoneNumber: response.phone_number, + }); return response; } catch (error) { this.logger.error("Failed to import phone number", { error }); throw error; } }packages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.ts (1)
81-91: Consider using a more robust unique identifier for agent names.Using
Date.now()for uniqueness could lead to collisions if multiple agents are created in the same millisecond. Consider using a UUID or nanoid library instead.+import { randomUUID } from 'crypto'; + static mapToCreateAgentRequest( llmId: string, eventTypeId?: number, voiceId = "11labs-Adrian" ): CreateAgentRequest { return { response_engine: { llm_id: llmId, type: "retell-llm" }, - agent_name: `agent-${eventTypeId || "default"}-${Date.now()}`, + agent_name: `agent-${eventTypeId || "default"}-${randomUUID()}`, voice_id: voiceId, }; }packages/features/ee/cal-ai-phone/providers/retell-ai/types.ts (1)
44-80: Consider using const assertion for the Language typeThe Language union type could benefit from using a const assertion pattern for better type safety and maintainability.
Consider refactoring to:
const LANGUAGES = [ "en-US", "en-IN", "en-GB", "en-AU", "en-NZ", "de-DE", "es-ES", "es-419", "hi-IN", "fr-FR", // ... rest of languages ] as const; export type Language = typeof LANGUAGES[number];This approach provides better IDE support and ensures the list is maintained in one place.
packages/prisma/migrations/20250806084401_add_cal_ai_self_serve/migration.sql (1)
52-89: Consider adding composite indexes for common query patternsBased on the service implementation, you'll likely query by
(userId, enabled)and(teamId, enabled)combinations frequently.Add composite indexes:
CREATE INDEX "Agent_userId_enabled_idx" ON "Agent"("userId", "enabled"); CREATE INDEX "Agent_teamId_enabled_idx" ON "Agent"("teamId", "enabled"); CREATE INDEX "CalAiPhoneNumber_userId_subscriptionStatus_idx" ON "CalAiPhoneNumber"("userId", "subscriptionStatus"); CREATE INDEX "CalAiPhoneNumber_teamId_subscriptionStatus_idx" ON "CalAiPhoneNumber"("teamId", "subscriptionStatus");packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts (1)
810-813: Consider team-scoped rate limitingThe rate limit identifier only uses userId. For team operations, consider including teamId to prevent one team from affecting another's rate limits.
await checkRateLimitAndThrowError({ rateLimitingType: "core", - identifier: `test-call:${userId}`, + identifier: teamId ? `test-call:team:${teamId}` : `test-call:user:${userId}`, });packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts (1)
50-75: Document the purpose of nullable fieldsSeveral fields in
AIPhoneServiceAgentListItemhave complex nullable types. Consider adding JSDoc comments explaining when these fields might be null or undefined.export interface AIPhoneServiceAgentListItem { id: string; name: string; retellAgentId: string; enabled: boolean; userId: number; + /** Team ID if the agent belongs to a team, null for personal agents */ teamId: number | null; createdAt: Date; updatedAt: Date; outboundPhoneNumbers: { id: number; phoneNumber: string; + /** Subscription status from Stripe, null if not subscribed */ subscriptionStatus: string | null; + /** Provider name, null if using default provider */ provider: string | null; }[];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (25)
apps/web/package.json(2 hunks)packages/features/ee/cal-ai-phone/README.md(1 hunks)packages/features/ee/cal-ai-phone/ai-phone-service-registry.test.ts(1 hunks)packages/features/ee/cal-ai-phone/ai-phone-service-registry.ts(1 hunks)packages/features/ee/cal-ai-phone/index.ts(1 hunks)packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/client.test.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/client.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/errors.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/factory.test.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/factory.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/index.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/provider.test.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/types.ts(1 hunks)packages/features/ee/cal-ai-phone/template-fields-map.ts(1 hunks)packages/features/ee/cal-ai-phone/zod-utils.ts(2 hunks)packages/lib/constants.ts(1 hunks)packages/lib/server/repository/agent.ts(1 hunks)packages/lib/server/repository/phoneNumber.ts(1 hunks)packages/prisma/migrations/20250806084401_add_cal_ai_self_serve/migration.sql(1 hunks)packages/prisma/schema.prisma(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/ee/cal-ai-phone/template-fields-map.tspackages/features/ee/cal-ai-phone/providers/retell-ai/errors.tspackages/features/ee/cal-ai-phone/providers/retell-ai/index.tspackages/features/ee/cal-ai-phone/providers/retell-ai/factory.test.tspackages/lib/constants.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/client.test.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.tspackages/features/ee/cal-ai-phone/providers/retell-ai/factory.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.tspackages/features/ee/cal-ai-phone/providers/retell-ai/client.tspackages/lib/server/repository/phoneNumber.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.test.tspackages/features/ee/cal-ai-phone/zod-utils.tspackages/features/ee/cal-ai-phone/providers/retell-ai/types.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.tspackages/features/ee/cal-ai-phone/index.tspackages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.tspackages/lib/server/repository/agent.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.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/ee/cal-ai-phone/template-fields-map.tspackages/features/ee/cal-ai-phone/providers/retell-ai/errors.tspackages/features/ee/cal-ai-phone/providers/retell-ai/index.tspackages/features/ee/cal-ai-phone/providers/retell-ai/factory.test.tspackages/lib/constants.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/client.test.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.tspackages/features/ee/cal-ai-phone/providers/retell-ai/factory.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.tspackages/features/ee/cal-ai-phone/providers/retell-ai/client.tspackages/lib/server/repository/phoneNumber.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.test.tspackages/features/ee/cal-ai-phone/zod-utils.tspackages/features/ee/cal-ai-phone/providers/retell-ai/types.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.tspackages/features/ee/cal-ai-phone/index.tspackages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.tspackages/lib/server/repository/agent.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
📚 Learning: in the cal.com cal ai phone feature, the deletephonenumber repository method validation is properly ...
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/index.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/client.test.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.tspackages/features/ee/cal-ai-phone/providers/retell-ai/client.tspackages/lib/server/repository/phoneNumber.tspackages/prisma/schema.prismapackages/features/ee/cal-ai-phone/providers/retell-ai/service.test.tspackages/features/ee/cal-ai-phone/README.mdpackages/features/ee/cal-ai-phone/providers/retell-ai/provider.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/types.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.tspackages/features/ee/cal-ai-phone/index.tspackages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.tspackages/lib/server/repository/agent.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/prisma/migrations/20250806084401_add_cal_ai_self_serve/migration.sql
📚 Learning: applies to **/*.ts : ensure the `credential.key` field is never returned from trpc endpoints or apis...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : Ensure the `credential.key` field is never returned from tRPC endpoints or APIs
Applied to files:
packages/lib/constants.ts
📚 Learning: cal.com integrations: `webapp_url_for_oauth` intentionally falls back to `http://localhost:3000` whe...
Learnt from: hariombalhara
PR: calcom/cal.com#22840
File: packages/app-store/webex/api/add.ts:4-4
Timestamp: 2025-08-04T13:14:39.218Z
Learning: Cal.com integrations: `WEBAPP_URL_FOR_OAUTH` intentionally falls back to `http://localhost:3000` when neither production nor development, matching the pattern used by other apps in the repo.
Applied to files:
packages/lib/constants.ts
📚 Learning: applies to **/*repository.ts : repository files must include `repository` suffix, prefix with techno...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Repository.ts : Repository files must include `Repository` suffix, prefix with technology if applicable (e.g., `PrismaAppRepository.ts`), and use PascalCase matching the exported class
Applied to files:
packages/lib/server/repository/phoneNumber.tspackages/lib/server/repository/agent.ts
📚 Learning: applies to docs/api-reference/v2/openapi.json : when docs changes are made in `/docs/api-reference/v...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to docs/api-reference/v2/openapi.json : When docs changes are made in `/docs/api-reference/v2/openapi.json`, ensure 'summary' fields are short, concise, do not end with a period, and are written in proper American English
Applied to files:
packages/features/ee/cal-ai-phone/README.md
📚 Learning: applies to **/*service.ts : service files must include `service` suffix, use pascalcase matching exp...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Service.ts : Service files must include `Service` suffix, use PascalCase matching exported class, and avoid generic names (e.g., `MembershipService.ts`)
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
🪛 markdownlint-cli2 (0.17.2)
packages/features/ee/cal-ai-phone/README.md
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
318-318: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
416-416: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
446-446: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
679-679: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (38)
apps/web/package.json (2)
3-3: Verify the package version downgrade.The version was downgraded from 5.5.13 to 5.5.11. Package versions typically move forward, not backward. Please confirm this downgrade is intentional and explain the reasoning.
141-141: LGTM! Retell SDK dependency addition is appropriate.The
retell-sdkdependency aligns with the AI phone service integration requirements and supports the Retell AI provider implementation.packages/features/ee/cal-ai-phone/template-fields-map.ts (1)
4-4: LGTM! Improved naming convention.The rename from
TEMPLATES_FIELDStotemplateFieldsMapfollows better camelCase naming conventions for exported objects that aren't constants.packages/lib/constants.ts (1)
24-27: LGTM! Proper server-only API key handling.The
RETELL_API_KEYconstant follows security best practices with clear server-only documentation and proper environment variable sourcing. This aligns with the existing patterns in the codebase.packages/features/ee/cal-ai-phone/providers/retell-ai/errors.ts (1)
1-6: LGTM! Well-designed custom error class.The
RetellAIErrorclass follows error handling best practices:
- Properly extends
Errorwithsuper()call- Provides contextual information with
operationparameter- Preserves original error for debugging
- Uses readonly properties for immutability
- Sets appropriate error name
packages/features/ee/cal-ai-phone/providers/retell-ai/client.test.ts (1)
1-412: LGTM! Comprehensive and well-structured test suite.This test file demonstrates excellent testing practices:
- Comprehensive coverage of all client methods (CRUD operations for LLMs, agents, phone numbers, and calls)
- Proper mocking and isolation with
beforeEachcleanup- Tests both success paths and error scenarios
- Includes edge cases (special characters in phone numbers, undefined dynamic variables)
- Clear test organization and descriptive test names
- Proper async/await handling throughout
The test quality supports robust validation of the Retell SDK client integration.
packages/features/ee/cal-ai-phone/zod-utils.ts (1)
177-201: Well-structured response schemas.The new response schemas for phone number and agent operations are comprehensive and properly typed. Good use of optional/nullable fields and proper URL validation for webhook URLs.
packages/features/ee/cal-ai-phone/providers/retell-ai/factory.test.ts (1)
1-123: Excellent test coverage and structure.The test suite demonstrates good practices:
- Comprehensive mocking with proper isolation
- Coverage of both logging enabled/disabled scenarios
- Verification of dependency injection
- Clean test structure with proper setup/teardown
packages/features/ee/cal-ai-phone/providers/retell-ai/provider.test.ts (2)
17-43: Good use of mock helper function.The
createMockRetellAIServicehelper function promotes consistency and maintainability across tests by providing a standardized way to create service mocks.
550-566: Proper error handling verification.Good practice to verify that errors from the service layer are properly propagated through the provider layer without modification.
packages/features/ee/cal-ai-phone/providers/retell-ai/factory.ts (2)
11-32: Clean factory implementation.Good implementation of the factory pattern with:
- Conditional logging support
- Both instance and static creation methods
- Proper dependency injection pattern
- Clear configuration handling
17-17: Factory usage is correct:RetellSDKClientsatisfiesRetellAIRepositoryI verified that
RetellSDKClientimplements the fullRetellAIRepositoryinterface, so passing it as the sole constructor argument aligns with the provider’s signature (constructor(repository: RetellAIRepository, service?: RetellAIService)). No changes needed here.packages/features/ee/cal-ai-phone/providers/retell-ai/index.ts (2)
1-16: Well-organized module exports.Clean separation of class exports and type exports with comprehensive coverage of the Retell AI provider API surface.
17-99: Excellent usage documentation.The commented examples provide comprehensive guidance for both new provider pattern and legacy service usage, including proper error handling patterns. This will be valuable for developers integrating the AI phone service.
packages/features/ee/cal-ai-phone/ai-phone-service-registry.test.ts (2)
1-28: LGTM! Clean mock setup.The mocking strategy properly isolates the registry tests from external dependencies.
206-359: LGTM! Comprehensive test coverage for helper functions.The tests properly verify configuration merging, default values, and provider creation logic.
packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts (4)
1-122: LGTM! Excellent mock setup and test initialization.The mocking strategy comprehensively covers all dependencies including Prisma transactions, repositories, Stripe, and credit services.
220-266: LGTM! Proper validation testing for phone number deletion.The tests correctly verify that the service layer handles validation of phone number status before deletion, which aligns with the architectural pattern of keeping validation in the service layer.
268-379: Excellent transaction and rollback testing!The tests thoroughly cover transaction scenarios including successful import, agent permission validation, and rollback with cleanup when database operations fail. This ensures data consistency in failure scenarios.
687-859: LGTM! Comprehensive credit system and rate limiting tests.The tests thoroughly cover credit validation, rate limiting, and various error scenarios including insufficient credits, missing phone numbers, and agent permission checks.
packages/features/ee/cal-ai-phone/ai-phone-service-registry.ts (1)
75-94: LGTM! Well-designed helper functions.The configuration merging and default values are appropriate, with proper environment-based logging configuration.
packages/lib/server/repository/agent.ts (4)
33-53: LGTM! Secure access control helpers.The helper methods properly use parameterized queries to prevent SQL injection and efficiently fetch team access information.
192-206: Excellent N+1 query prevention!Good performance optimization by fetching phone numbers in a separate query rather than using joins that could cause N+1 issues.
358-454: LGTM! Pragmatic mix of raw SQL and Prisma ORM.Good architectural decision to use Prisma ORM for simple CRUD operations while using raw SQL for complex queries with performance considerations.
456-473: LGTM! Efficient permission checking.The method efficiently checks team management permissions with a single COUNT query.
packages/features/ee/cal-ai-phone/README.md (1)
1-765: Excellent comprehensive documentation!The README provides thorough coverage of architecture, design patterns, usage examples, and integration guides. This will be invaluable for developers working with the AI phone service.
packages/features/ee/cal-ai-phone/providers/retell-ai/client.ts (2)
22-32: LGTM! Proper initialization with validation.The constructor correctly validates the API key presence and initializes the SDK client with appropriate error handling.
34-48: LGTM! Well-structured method with proper error handling.The method correctly logs the operation details and handles errors appropriately by logging and re-throwing them.
packages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.ts (1)
27-62: LGTM! Well-structured tool building logic.The method correctly builds the tools array with appropriate conditional logic for calendar tools and proper handling of additional tools.
packages/features/ee/cal-ai-phone/index.ts (1)
1-92: LGTM! Well-organized module exports with helpful examples.The index file properly exports all necessary types, interfaces, and implementations. The inclusion of legacy exports for backward compatibility and detailed usage examples in comments is excellent for developer experience.
packages/prisma/schema.prisma (2)
2424-2432: LGTM! Standard subscription status enum.The enum correctly defines standard Stripe subscription statuses for phone number billing integration.
1326-1327: LGTM! Workflow integration properly implemented.The additions to WorkflowActions, WorkflowMethods, and WorkflowStep correctly integrate AI phone call functionality into the existing workflow system.
Also applies to: 1345-1346, 1484-1484
packages/features/ee/cal-ai-phone/providers/retell-ai/types.ts (1)
10-42: Well-structured discriminated unions and type guards!The discriminated union pattern with corresponding type guards provides excellent type safety for different response engine types.
packages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts (2)
21-26: Clean dependency injection pattern!The constructor properly implements dependency injection with optional service parameter, allowing for easy testing and flexibility.
28-45: Well-structured configuration managementThe setup and deletion methods properly map between the generic interface and Retell-specific types with clear return value transformations.
Also applies to: 47-61
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts (2)
89-111: Excellent transaction handling with cleanup!The importPhoneNumber method demonstrates proper transaction usage with rollback and cleanup of external resources on failure.
256-309: Proper validation in service layerAs per the retrieved learning, the deletePhoneNumber method correctly handles user ownership and authorization validation at the service layer before calling repository methods.
packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts (1)
275-278: Good use of enum for provider types!The enum pattern for provider types allows for easy extension while maintaining type safety.
packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts
Outdated
Show resolved
Hide resolved
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
Outdated
Show resolved
Hide resolved
packages/prisma/migrations/20250806084401_add_cal_ai_self_serve/migration.sql
Outdated
Show resolved
Hide resolved
packages/prisma/migrations/20250806084401_add_cal_ai_self_serve/migration.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (6)
packages/prisma/migrations/20250806103638_add_cal_ai_self_serve/migration.sql (2)
19-31: Add constraint to ensure either userId OR teamId is set.The Agent table allows both userId and teamId to be null or both to be set, which could lead to data integrity issues. Consider adding a check constraint to ensure exactly one is set.
Add a check constraint in your migration:
ALTER TABLE "Agent" ADD CONSTRAINT "Agent_owner_check" CHECK ( (("userId" IS NOT NULL)::integer + ("teamId" IS NOT NULL)::integer) = 1 );
33-50: Add constraint to ensure either userId OR teamId is set.Similar to the Agent table, CalAiPhoneNumber should have exactly one owner (either user or team).
Add a check constraint in your migration:
ALTER TABLE "CalAiPhoneNumber" ADD CONSTRAINT "CalAiPhoneNumber_owner_check" CHECK ( (("userId" IS NOT NULL)::integer + ("teamId" IS NOT NULL)::integer) = 1 );packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts (1)
536-542: Add proper type guard for error checking.The error type assertion
(error as Error)is unsafe and could throw if error is not an Error object.Use a proper type guard:
} catch (error: unknown) { // Check if it's a 404 error (phone number not found in Retell) - if ((error as Error).message?.includes("404") || (error as Error).message?.includes("Not Found")) { + const errorMessage = error instanceof Error ? error.message : String(error); + if (errorMessage.includes("404") || errorMessage.includes("Not Found")) { console.log(`Phone number ${phoneNumber} not found in Retell - updating local database only`); } else { console.error("Failed to update phone number in AI service:", error); }packages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts (1)
158-165: MissingteamIdparameter inupdatePhoneNumberWithAgentsmethodThe interface likely includes an optional
teamIdparameter that should be passed to the service for proper access control.async updatePhoneNumberWithAgents(params: { phoneNumber: string; userId: number; + teamId?: number; inboundAgentId?: string | null; outboundAgentId?: string | null; }): Promise<{ message: string }> { return await this.service.updatePhoneNumberWithAgents(params); }packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts (1)
1-40: Architectural consideration: Interface layer imports from provider implementationsWhile this creates a dependency from the abstraction to implementations (violating traditional DIP), the documented approach provides strong type safety through compile-time type mapping. This is a reasonable trade-off for TypeScript projects where type safety is prioritized.
Consider documenting this architectural decision more prominently, perhaps in the README or architecture documentation, to explain why this approach was chosen over traditional interface segregation.
packages/features/ee/cal-ai-phone/ai-phone-service-registry.ts (1)
75-95: API key validation properly implemented.The function correctly validates the API key and provides a clear error message when it's missing, addressing the previous review feedback. The configuration merging is also handled correctly with the spread operator.
🧹 Nitpick comments (3)
packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts (1)
318-323: Improve logger type definitionThe
loggerfield usesanytype which bypasses TypeScript's type checking. Consider using a proper logger interface or at leastunknownfor better type safety.export interface AIPhoneServiceProviderConfig { apiKey?: string; baseUrl?: string; enableLogging?: boolean; - logger?: any; // Logger instance + logger?: unknown; // Logger instance - consider defining a Logger interface }Or better yet, define a minimal logger interface:
interface Logger { debug(...args: any[]): void; info(...args: any[]): void; warn(...args: any[]): void; error(...args: any[]): void; }packages/features/ee/cal-ai-phone/ai-phone-service-registry.ts (2)
19-21: Consider adding protection against overwriting existing providers.The
registerProvidermethod doesn't check if a provider is already registered, which could lead to accidental overwrites.Consider adding a check to prevent unintentional overwrites:
static registerProvider(type: string, factory: AIPhoneServiceProviderFactory): void { + if (this.factories.has(type)) { + console.warn(`Provider '${type}' is already registered and will be overwritten`); + } this.factories.set(type, factory); }Or alternatively, throw an error if you want to enforce stricter registration:
static registerProvider(type: string, factory: AIPhoneServiceProviderFactory): void { + if (this.factories.has(type)) { + throw new Error(`Provider '${type}' is already registered`); + } this.factories.set(type, factory); }
73-73: Consider lazy initialization for better testability.Registering the provider at module load time creates a side effect that could complicate testing. Tests might need to clear and re-register providers.
Consider lazy initialization or an explicit initialization method:
// Option 1: Lazy initialization private static ensureDefaultProviderRegistered(): void { if (!this.factories.has(AIPhoneServiceProviderType.RETELL_AI)) { this.registerProvider(AIPhoneServiceProviderType.RETELL_AI, new RetellAIProviderFactory()); } } // Call this in createProvider before checking factory existenceOr keep it as is but document the testing implications, since you already have the
clearProviders()method for tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
packages/app-store/stripepayment/lib/constants.ts(1 hunks)packages/app-store/stripepayment/lib/utils.ts(2 hunks)packages/features/ee/cal-ai-phone/ai-phone-service-registry.ts(1 hunks)packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/factory.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/index.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/provider.test.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts(1 hunks)packages/features/ee/workflows/lib/constants.ts(1 hunks)packages/features/eventtypes/components/tabs/ai/AIEventController.tsx(2 hunks)packages/lib/server/repository/agentRepository.ts(1 hunks)packages/lib/server/repository/phoneNumberRespository.ts(1 hunks)packages/prisma/migrations/20250806103638_add_cal_ai_self_serve/migration.sql(1 hunks)packages/prisma/migrations/20250806104007_add_agent_phone_number_owner_check_constraints/migration.sql(1 hunks)packages/prisma/schema.prisma(6 hunks)packages/trpc/server/routers/viewer/workflows/update.handler.ts(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- packages/features/eventtypes/components/tabs/ai/AIEventController.tsx
- packages/app-store/stripepayment/lib/utils.ts
- packages/features/ee/workflows/lib/constants.ts
- packages/app-store/stripepayment/lib/constants.ts
- packages/prisma/migrations/20250806104007_add_agent_phone_number_owner_check_constraints/migration.sql
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/features/ee/cal-ai-phone/providers/retell-ai/index.ts
- packages/features/ee/cal-ai-phone/providers/retell-ai/provider.test.ts
- packages/features/ee/cal-ai-phone/providers/retell-ai/factory.ts
- packages/features/ee/cal-ai-phone/providers/retell-ai/service-mappers.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/trpc/server/routers/viewer/workflows/update.handler.tspackages/lib/server/repository/phoneNumberRespository.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.test.tspackages/lib/server/repository/agentRepository.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.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/trpc/server/routers/viewer/workflows/update.handler.tspackages/lib/server/repository/phoneNumberRespository.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.test.tspackages/lib/server/repository/agentRepository.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts
**/*Repository.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Repository files must include
Repositorysuffix, prefix with technology if applicable (e.g.,PrismaAppRepository.ts), and use PascalCase matching the exported class
Files:
packages/lib/server/repository/agentRepository.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
📚 Learning: in the cal.com cal ai phone feature, the deletephonenumber repository method validation is properly ...
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
Applied to files:
packages/lib/server/repository/phoneNumberRespository.tspackages/features/ee/cal-ai-phone/ai-phone-service-registry.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.test.tspackages/lib/server/repository/agentRepository.tspackages/prisma/migrations/20250806103638_add_cal_ai_self_serve/migration.sqlpackages/features/ee/cal-ai-phone/providers/retell-ai/provider.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.tspackages/prisma/schema.prisma
📚 Learning: applies to **/*repository.ts : repository files must include `repository` suffix, prefix with techno...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Repository.ts : Repository files must include `Repository` suffix, prefix with technology if applicable (e.g., `PrismaAppRepository.ts`), and use PascalCase matching the exported class
Applied to files:
packages/lib/server/repository/phoneNumberRespository.tspackages/lib/server/repository/agentRepository.ts
📚 Learning: applies to **/*.ts : ensure the `credential.key` field is never returned from trpc endpoints or apis...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : Ensure the `credential.key` field is never returned from tRPC endpoints or APIs
Applied to files:
packages/features/ee/cal-ai-phone/ai-phone-service-registry.ts
📚 Learning: applies to **/*service.ts : service files must include `service` suffix, use pascalcase matching exp...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Service.ts : Service files must include `Service` suffix, use PascalCase matching exported class, and avoid generic names (e.g., `MembershipService.ts`)
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
📚 Learning: in signup-view.tsx, when checking if redirecturl contains certain strings, using explicit && checks ...
Learnt from: Anshumancanrock
PR: calcom/cal.com#22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
📚 Learning: for large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, fo...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
📚 Learning: applies to **/*.{service,repository}.ts : avoid dot-suffixes like `.service.ts` or `.repository.ts` ...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{service,repository}.ts : Avoid dot-suffixes like `.service.ts` or `.repository.ts` for new files; reserve `.test.ts`, `.spec.ts`, `.types.ts` for their specific purposes
Applied to files:
packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts
📚 Learning: in cal.com's schema design, the team prefers to keep boolean fields nullable (boolean?) with default...
Learnt from: vijayraghav-io
PR: calcom/cal.com#16579
File: packages/prisma/schema.prisma:149-153
Timestamp: 2025-07-16T07:14:49.248Z
Learning: In Cal.com's schema design, the team prefers to keep Boolean fields nullable (Boolean?) with defaults rather than making them non-nullable (Boolean) to avoid expensive database migrations that would update existing rows.
Applied to files:
packages/prisma/schema.prisma
🧬 Code Graph Analysis (2)
packages/trpc/server/routers/viewer/workflows/update.handler.ts (1)
packages/trpc/server/routers/viewer/workflows/util.ts (1)
isStepEdited(824-839)
packages/lib/server/repository/agentRepository.ts (1)
packages/platform/libraries/index.ts (1)
MembershipRole(98-98)
⏰ Context from checks skipped due to timeout of 90000ms. 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 (11)
packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts (1)
1-860: Excellent test coverage!The test suite is comprehensive and well-structured, covering success cases, error scenarios, permission checks, transaction rollbacks, and edge cases. The mock setup is clean and the tests are well-organized.
packages/lib/server/repository/agentRepository.ts (1)
1-474: Well-structured repository with comprehensive access controlThe
AgentRepositoryimplementation is excellent with proper access control, optimized queries to avoid N+1 problems, and appropriate use of both raw SQL and Prisma ORM. The code follows best practices for repository pattern implementation.packages/prisma/schema.prisma (1)
2435-2492: Schema changes align with migration.The schema additions for Agent and CalAiPhoneNumber models are well-structured with appropriate relations and indexes. The ownership constraint issues mentioned in the migration review apply here as well.
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts (3)
39-56: LGTM! Clean AI configuration setup.The method properly handles defaults and maintains a clear separation between LLM and Agent creation.
58-153: Excellent transaction handling and cleanup logic.The method properly validates permissions, uses transactions for data consistency, and includes cleanup logic for Retell resources on failure. Good separation of concerns with pre-transaction permission checks.
309-383: Well-implemented Stripe checkout session creation.The method properly handles configuration checks, customer creation, and includes comprehensive metadata for tracking. Good use of IS_PRODUCTION flag for automatic tax configuration.
packages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts (2)
110-122: Previous issue resolved:teamIdparameter properly handledThe
teamIdparameter is now correctly included in the method signature and passed to the service layer.
150-156: Previous issue resolved:teamIdparameter properly handledThe
teamIdparameter is now correctly included in the params object and passed to the service layer.packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts (1)
140-313: Well-designed provider interfaceThe
AIPhoneServiceProviderinterface is well-structured with:
- Comprehensive documentation for each method
- Consistent parameter patterns for authorization
- Proper use of generics for type safety
- Clear separation of concerns
packages/features/ee/cal-ai-phone/ai-phone-service-registry.ts (2)
1-10: Imports are well-organized and appropriate.The imports follow a logical structure with constants, interfaces, and concrete implementations properly separated.
97-101: Clean delegation to the main factory function.The helper function provides a convenient way to create the default provider while maintaining a single source of truth for the creation logic.
packages/features/ee/cal-ai-phone/interfaces/ai-phone-service.interface.ts
Outdated
Show resolved
Hide resolved
packages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts
Outdated
Show resolved
Hide resolved
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
Outdated
Show resolved
Hide resolved
packages/trpc/server/routers/viewer/workflows/update.handler.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts (2)
536-542: Add proper type guard for error checkingThe error type assertion
(error as Error)is unsafe and could throw if error is not an Error object.Use a proper type guard:
} catch (error: unknown) { // Check if it's a 404 error (phone number not found in Retell) - if ((error as Error).message?.includes("404") || (error as Error).message?.includes("Not Found")) { + const errorMessage = error instanceof Error ? error.message : String(error); + if (errorMessage.includes("404") || errorMessage.includes("Not Found")) { console.log(`Phone number ${phoneNumber} not found in Retell - updating local database only`); } else { console.error("Failed to update phone number in AI service:", error); }
1-843: Consider splitting this large service fileThis service file is over 840 lines, making it difficult to maintain and test. Per best practices for large PRs, consider splitting into smaller, focused services.
Suggested split:
RetellAIConfigurationService- AI setup/deletion operationsRetellAIPhoneNumberService- Phone number operationsRetellAIAgentService- Agent operationsRetellAICallService- Call operationsRetellAIBillingService- Billing/subscription operationsWould you like me to create an issue to track this refactoring task?
packages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts (1)
159-166: MissingteamIdparameter inupdatePhoneNumberWithAgentsmethodThe interface includes an optional
teamIdparameter that should be passed to the service.async updatePhoneNumberWithAgents(params: { phoneNumber: string; userId: number; + teamId?: number; inboundAgentId?: string | null; outboundAgentId?: string | null; }): Promise<{ message: string }> { return await this.service.updatePhoneNumberWithAgents(params); }
🧹 Nitpick comments (1)
packages/lib/server/repository/phoneNumberRepository.ts (1)
225-344: Consider refactoring this complex method for better maintainabilityThis method is 119 lines long with complex conditional logic. Consider extracting the WHERE condition building and agent aggregation logic into separate private methods.
Split into smaller methods:
buildWhereCondition(userId, teamId, scope, accessibleTeamIds)- for building the WHERE clauseaggregateAgentsByPhoneNumber(agents)- for the agent aggregation logicenrichPhoneNumbersWithAgents(phoneNumbers, agentsByPhoneNumber)- for the final mappingThis would improve readability and make the code easier to test and maintain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts(1 hunks)packages/lib/server/repository/phoneNumberRepository.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*Repository.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Repository files must include
Repositorysuffix, prefix with technology if applicable (e.g.,PrismaAppRepository.ts), and use PascalCase matching the exported class
Files:
packages/lib/server/repository/phoneNumberRepository.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:
packages/lib/server/repository/phoneNumberRepository.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.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/server/repository/phoneNumberRepository.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
📚 Learning: in the cal.com cal ai phone feature, the deletephonenumber repository method validation is properly ...
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
Applied to files:
packages/lib/server/repository/phoneNumberRepository.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/features/ee/cal-ai-phone/providers/retell-ai/provider.ts
📚 Learning: applies to **/*repository.ts : repository files must include `repository` suffix, prefix with techno...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Repository.ts : Repository files must include `Repository` suffix, prefix with technology if applicable (e.g., `PrismaAppRepository.ts`), and use PascalCase matching the exported class
Applied to files:
packages/lib/server/repository/phoneNumberRepository.ts
📚 Learning: applies to **/*service.ts : service files must include `service` suffix, use pascalcase matching exp...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Service.ts : Service files must include `Service` suffix, use PascalCase matching exported class, and avoid generic names (e.g., `MembershipService.ts`)
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
📚 Learning: in signup-view.tsx, when checking if redirecturl contains certain strings, using explicit && checks ...
Learnt from: Anshumancanrock
PR: calcom/cal.com#22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
📚 Learning: for large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, fo...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
packages/lib/server/repository/phoneNumberRepository.ts (2)
185-192: Remove duplicate method definitionThis method
findByPhoneNumberAndUserIdis already defined at lines 38-45. The duplicate definition will override the first one.- static async findByPhoneNumberAndUserId({ phoneNumber, userId }: { phoneNumber: string; userId: number }) { - return await prisma.calAiPhoneNumber.findFirst({ - where: { - phoneNumber, - userId, - }, - }); - }
38-45: Useselectto fetch only needed fieldsPer coding guidelines, use
selectto explicitly specify needed fields instead of returning all columns.static async findByPhoneNumberAndUserId({ phoneNumber, userId }: { phoneNumber: string; userId: number }) { return await prisma.calAiPhoneNumber.findFirstOrThrow({ + select: { + id: true, + phoneNumber: true, + userId: true, + teamId: true, + subscriptionStatus: true, + stripeSubscriptionId: true, + stripeCustomerId: true, + provider: true, + inboundAgentId: true, + outboundAgentId: true, + createdAt: true, + updatedAt: true, + }, where: { phoneNumber, userId, }, }); }packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts (3)
1-844: Consider splitting this large service fileThis 844-line service file handles multiple responsibilities, making it difficult to maintain and test.
Consider splitting into focused services:
RetellAIConfigurationService- AI setup/deletion operationsRetellAIPhoneNumberService- Phone number operationsRetellAIAgentService- Agent operationsRetellAICallService- Call operationsRetellAIBillingService- Billing/subscription operationsWould you like me to create an issue to track this refactoring task?
165-189: Use structured error detection instead of string matchingChecking for "not found" or "404" in error messages is fragile and could break if message formats change.
Consider enhancing the RetellAIError class to include a statusCode property for more reliable error detection, then check the status code directly instead of parsing error messages.
536-542: Use proper type guard for error checkingThe error type assertion
(error as Error)is unsafe and could throw if error is not an Error object.} catch (error: unknown) { // Check if it's a 404 error (phone number not found in Retell) - if ((error as Error).message?.includes("404") || (error as Error).message?.includes("Not Found")) { + const errorMessage = error instanceof Error ? error.message : String(error); + if (errorMessage.includes("404") || errorMessage.includes("Not Found")) { console.log(`Phone number ${phoneNumber} not found in Retell - updating local database only`); } else { console.error("Failed to update phone number in AI service:", error); } }
🧹 Nitpick comments (1)
packages/lib/server/repository/phoneNumberRepository.ts (1)
365-423: Optimize agent lookups withselectThe agent lookups only need the
idfield for the connect operation.if (inboundRetellAgentId !== undefined) { if (inboundRetellAgentId) { const agent = await prisma.agent.findFirst({ + select: { + id: true, + }, where: { retellAgentId: inboundRetellAgentId, }, }); if (agent) { updateData.inboundAgent = { connect: { id: agent.id }, }; } else { updateData.inboundAgent = { disconnect: true }; } } else { updateData.inboundAgent = { disconnect: true }; } } if (outboundRetellAgentId !== undefined) { if (outboundRetellAgentId) { const agent = await prisma.agent.findFirst({ + select: { + id: true, + }, where: { retellAgentId: outboundRetellAgentId, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts(1 hunks)packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts(1 hunks)packages/features/ee/workflows/components/WorkflowDetailsPage.tsx(1 hunks)packages/lib/server/repository/phoneNumberRepository.ts(1 hunks)packages/trpc/server/routers/viewer/workflows/update.handler.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/features/ee/workflows/components/WorkflowDetailsPage.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/trpc/server/routers/viewer/workflows/update.handler.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/features/ee/cal-ai-phone/providers/retell-ai/service.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/lib/server/repository/phoneNumberRepository.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/ee/cal-ai-phone/providers/retell-ai/service.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/lib/server/repository/phoneNumberRepository.ts
**/*Repository.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Repository files must include
Repositorysuffix, prefix with technology if applicable (e.g.,PrismaAppRepository.ts), and use PascalCase matching the exported class
Files:
packages/lib/server/repository/phoneNumberRepository.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
📚 Learning: in the cal.com cal ai phone feature, the deletephonenumber repository method validation is properly ...
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.tspackages/features/ee/cal-ai-phone/providers/retell-ai/service.tspackages/lib/server/repository/phoneNumberRepository.ts
📚 Learning: applies to **/*service.ts : service files must include `service` suffix, use pascalcase matching exp...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Service.ts : Service files must include `Service` suffix, use PascalCase matching exported class, and avoid generic names (e.g., `MembershipService.ts`)
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
📚 Learning: in signup-view.tsx, when checking if redirecturl contains certain strings, using explicit && checks ...
Learnt from: Anshumancanrock
PR: calcom/cal.com#22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
📚 Learning: for large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, fo...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Applied to files:
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts
📚 Learning: applies to **/*repository.ts : repository files must include `repository` suffix, prefix with techno...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Repository.ts : Repository files must include `Repository` suffix, prefix with technology if applicable (e.g., `PrismaAppRepository.ts`), and use PascalCase matching the exported class
Applied to files:
packages/lib/server/repository/phoneNumberRepository.ts
📚 Learning: applies to **/*.ts : for prisma queries, only select data you need; never use `include`, always use ...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : For Prisma queries, only select data you need; never use `include`, always use `select`
Applied to files:
packages/lib/server/repository/phoneNumberRepository.ts
🪛 Biome (2.1.2)
packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts
[error] 12-12: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
packages/lib/server/repository/phoneNumberRepository.ts
[error] 185-192: Duplicate class member name "findByPhoneNumberAndUserId"
(lint/suspicious/noDuplicateClassMembers)
⏰ Context from checks skipped due to timeout of 90000ms. 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/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts (6)
77-122: Well-structured test setupThe test suite setup properly initializes mocks, handles transaction simulation, and ensures clean state between tests.
123-168: Comprehensive test coverage for AI configuration setupThe tests properly validate both minimal and extended configuration scenarios, including Cal.com tool integration.
170-218: Thorough deletion scenario testingExcellent coverage of deletion edge cases including success, 404 errors, and partial failures with proper error aggregation.
220-266: Proper validation of phone number deletion constraintsTests correctly validate business rules preventing deletion of active or cancelled phone numbers, properly implementing service-layer validation as per architectural patterns.
268-379: Excellent transactional integrity testingComprehensive coverage of phone number import including transaction rollback, permission checks, and cleanup on failure. The database error simulation and rollback verification are particularly well-implemented.
687-859: Exceptional test coverage for createTestCallComprehensive validation of all edge cases including credit checks, rate limiting, agent permissions, and phone number requirements. The null/undefined credit handling is particularly valuable for robustness.
packages/lib/server/repository/phoneNumberRepository.ts (4)
47-121: Well-implemented complex query with proper SQL injection preventionThe method efficiently fetches phone numbers with their associated agents using safe parameterized queries and proper data transformation.
124-152: Properly focused data access methodsThese methods correctly focus on data operations without business logic, aligning with the architectural pattern where validation happens in the service layer.
217-336: Excellent implementation of complex access-controlled queryWell-structured method with proper SQL parameterization, efficient querying, and comprehensive access control based on scope.
338-363: Clean subscription status update implementationWell-structured method with proper conditional logic for optional agent disconnection.
packages/features/ee/cal-ai-phone/providers/retell-ai/service.ts (4)
58-153: Excellent transactional implementation with proper cleanupWell-structured method with pre-transaction validation, proper rollback handling, and cleanup of external resources on failure.
243-296: Proper service-layer validation implementationCorrectly implements business rule validation in the service layer, aligning with architectural best practices. Good error handling for agent removal failures.
770-842: Well-implemented test call creation with proper validationsComprehensive validation of credits, rate limiting, and agent permissions. The credit requirement constant is properly used throughout.
309-383: Well-structured Stripe integrationComprehensive checkout session creation with proper error handling, metadata tracking, and production-aware tax configuration.
packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/lib/server/repository/phoneNumberRepository.ts (1)
166-173: Applyselectpattern to comply with coding guidelines.The
findByIdmethod violates the coding guideline to useselectinstead of returning all fields.Apply this fix:
static async findById({ id, userId }: { id: number; userId: number }) { return await prisma.calAiPhoneNumber.findFirst({ + select: { + id: true, + phoneNumber: true, + userId: true, + teamId: true, + subscriptionStatus: true, + stripeSubscriptionId: true, + stripeCustomerId: true, + provider: true, + inboundAgentId: true, + outboundAgentId: true, + createdAt: true, + updatedAt: true, + }, where: { id, userId, }, }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts(1 hunks)packages/lib/server/repository/phoneNumberRepository.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/features/ee/cal-ai-phone/providers/retell-ai/service.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*Repository.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Repository files must include
Repositorysuffix, prefix with technology if applicable (e.g.,PrismaAppRepository.ts), and use PascalCase matching the exported class
Files:
packages/lib/server/repository/phoneNumberRepository.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:
packages/lib/server/repository/phoneNumberRepository.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/server/repository/phoneNumberRepository.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
📚 Learning: in the cal.com cal ai phone feature, the deletephonenumber repository method validation is properly ...
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
Applied to files:
packages/lib/server/repository/phoneNumberRepository.ts
📚 Learning: applies to **/*repository.ts : repository files must include `repository` suffix, prefix with techno...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Repository.ts : Repository files must include `Repository` suffix, prefix with technology if applicable (e.g., `PrismaAppRepository.ts`), and use PascalCase matching the exported class
Applied to files:
packages/lib/server/repository/phoneNumberRepository.ts
📚 Learning: applies to **/*.ts : for prisma queries, only select data you need; never use `include`, always use ...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : For Prisma queries, only select data you need; never use `include`, always use `select`
Applied to files:
packages/lib/server/repository/phoneNumberRepository.ts
📚 Learning: in signup-view.tsx, when checking if redirecturl contains certain strings, using explicit && checks ...
Learnt from: Anshumancanrock
PR: calcom/cal.com#22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Applied to files:
packages/lib/server/repository/phoneNumberRepository.ts
🪛 Biome (2.1.2)
packages/lib/server/repository/phoneNumberRepository.ts
[error] 212-219: Duplicate class member name "findByPhoneNumberAndUserId"
(lint/suspicious/noDuplicateClassMembers)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Detect changes
🔇 Additional comments (6)
packages/lib/server/repository/phoneNumberRepository.ts (6)
1-26: LGTM! Clean imports and interface definitions.The imports are appropriate and the interface definitions for raw query results are well-structured.
28-36: LGTM! Proper access control utility method.The raw SQL approach is appropriate for this team membership query and uses proper parameterization for security.
61-135: LGTM! Well-implemented complex aggregation method.The
findPhoneNumbersFromUserIdmethod effectively uses raw SQL for complex joins and properly aggregates agent data with phone numbers. The conditional query execution and result mapping are handled correctly.
244-363: LGTM! Comprehensive access control with proper data aggregation.The
findManyWithUserAccessmethod implements sophisticated scope-based access control and maintains consistent data aggregation patterns. The dynamic SQL building and team access validation are well-handled.
137-164: LGTM! Clean CRUD operations with proper separation of concerns.The create and delete operations are well-implemented and appropriately focus on data access, consistent with the repository pattern where validation is handled at the service layer.
365-449: LGTM! Sophisticated relationship management methods.The
updateSubscriptionStatusandupdateAgentsmethods properly handle complex Prisma relationship operations using connect/disconnect patterns. The conditional update logic is well-structured and handles edge cases appropriately.
packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts (3)
86-97: Do not log request payload; it may contain secrets (e.g., SIP trunk password).Avoid logging
data. Keep errors structured, but omit sensitive fields.try { return await this.retellRepository.createPhoneNumber(data); } catch (error) { - this.logger.error("Failed to create phone number in external AI service", { - data, - error, - }); + this.logger.error("Failed to create phone number in external AI service", { + // NOTE: request payload omitted to avoid leaking secrets + error, + }); throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", message: "Failed to create phone number", }); }
178-189: Avoid leaking identifiers in public error messages.The TRPCError message includes the phone number. Keep identifiers only in logs.
} catch (error) { this.logger.error("Failed to get phone number from external AI service", { phoneNumber, error, }); throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", - message: `Failed to get phone number '${phoneNumber}'`, + message: "Failed to get phone number", }); }
213-221: Same here: remove phone number from TRPCError message.Keep the identifier in logs only.
} catch (error) { this.logger.error("Failed to update phone number in external AI service", { phoneNumber, data, error, }); throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", - message: `Failed to update phone number '${phoneNumber}'`, + message: "Failed to update phone number", }); }
🧹 Nitpick comments (1)
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts (1)
50-76: Optional: Minimize time spent inside DB transactions.External API calls within
executeInTransactioncan prolong transactions and lock resources. If the transaction manager opens the DB tx at the start of the closure, consider:
- Performing Retell import before starting the DB transaction, then opening a short-lived transaction for DB writes.
- Keeping compensation logic as-is for cleanup on DB failure.
Confirm how
TransactionInterface.executeInTransactionscopes its DB transaction (e.g., starts immediately or lazily on first DB op). If it starts immediately, moving Retell calls out will reduce lock duration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/features/calAIPhone/providers/retellAI/RetellAIService.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/AgentService.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/CallService.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts
- packages/features/calAIPhone/providers/retellAI/services/CallService.ts
- packages/features/calAIPhone/providers/retellAI/services/AgentService.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*Service.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.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:
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.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/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.870Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.665Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
📚 Learning: 2025-08-05T13:17:23.491Z
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.ts
📚 Learning: 2025-08-08T09:29:11.665Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.665Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.ts
📚 Learning: 2025-08-08T09:27:23.870Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.870Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.ts
📚 Learning: 2025-08-08T09:12:08.269Z
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.269Z
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/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : Ensure the `credential.key` field is never returned from tRPC endpoints or APIs
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts
🔇 Additional comments (4)
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts (2)
161-165: Good fix: delete external resource before DB to avoid orphaned Retell numbers.Order now prevents dangling Retell resources if DB deletion fails.
59-66: No changes needed—parameter names intentionally distinguish internal vs. provider agent IDs
TheoutboundAgentIdin createPhoneNumber is the internal agent’s ID, whileinboundProviderAgentId/outboundProviderAgentIdin updateAgents are the external provider’s agent IDs. This naming matches thePhoneNumberRepositoryInterfaceand its implementations; you can safely ignore this comment.packages/features/calAIPhone/providers/retellAI/RetellAIService.ts (2)
62-67: Nice type tightening for LLM updates (provider-parameterized).Using AIPhoneServiceUpdateModelParams parameterized with the Retell provider type improves compile-time safety.
Also applies to: 171-175
77-99: Pass-throughs look correct and cohesive.Composition across services is clean; method contracts align with underlying services.
Also applies to: 101-109, 185-195, 197-208
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts
Show resolved
Hide resolved
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts (4)
160-164: Good fix: delete from Retell first, then conditionally remove DB recordThis ordering avoids orphaned Retell resources if external deletion fails. Nice.
76-79: Wrap non-HTTP errors after compensation to standardize client responsesRe-throwing the raw error produces inconsistent API responses. After compensating, wrap unknown errors into HttpError while preserving HttpError as-is.
- } catch (error) { - await this.handleCompensatingTransaction(transactionState, error, { userId, teamId, agentId }); - throw error; - } + } catch (error: unknown) { + await this.handleCompensatingTransaction(transactionState, error, { userId, teamId, agentId }); + if (error instanceof HttpError) { + throw error; + } + throw new HttpError({ + statusCode: 500, + message: "Failed to import phone number", + }); + }
82-96: Don’t log sensitive payloads; add early validationAvoid logging the full request payload (it may contain SIP trunk auth password). Also add a guard for missing data.
async createPhoneNumber( data: AIPhoneServiceCreatePhoneNumberParams ): Promise<AIPhoneServicePhoneNumber<AIPhoneServiceProviderType.RETELL_AI>> { - try { + if (!data) { + throw new HttpError({ statusCode: 400, message: "Phone number data is required" }); + } + try { return await this.retellRepository.createPhoneNumber(data); } catch (error) { - this.logger.error("Failed to create phone number in external AI service", { - data, - error, - }); + this.logger.error("Failed to create phone number in external AI service", { + error, + // NOTE: request payload omitted to avoid leaking secrets + }); throw new HttpError({ statusCode: 500, message: "Failed to create phone number", }); } }
373-393: Redact phone number from compensation failure message returned to clientsReturn a generic message; keep detailed phoneNumber only in logs.
- const compensationFailureMessage = `Failed to cleanup Retell phone number ${transactionState.retellPhoneNumber.phone_number} after transaction failure. Manual cleanup required.`; + const compensationFailureMessage = "Failed to cleanup Retell phone number after transaction failure. Manual cleanup required."; @@ throw new HttpError({ statusCode: 500, message: compensationFailureMessage, });packages/features/calAIPhone/providers/retellAI/services/AgentService.ts (3)
53-54: Type safety improvement looks goodUsing Language type instead of any enforces correctness.
162-162: Good change: use uuidv4 for unique agent namesThis avoids Math.random() collisions/non-crypto concerns.
200-288: Acknowledging intentional non-persistence of name fieldPer maintainer guidance, not persisting optional name in updateAgentConfiguration is intentional for now. No action needed.
🧹 Nitpick comments (1)
packages/features/calAIPhone/providers/retellAI/services/CallService.ts (1)
41-57: Consider redacting phone numbers in thrown error messagesYou already log from/to numbers. For client-facing errors, consider a generic message to reduce PII exposure. Note: this change will require updating tests expecting the full message.
} catch (error) { this.logger.error("Failed to create phone call in external AI service", { fromNumber: data.from_number, toNumber: data.to_number, error, }); throw new HttpError({ statusCode: 500, - message: `Failed to create phone call from ${data.from_number} to ${data.to_number}`, + message: "Failed to create phone call", }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/AgentService.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/BillingService.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/CallService.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/__tests__/BillingService.test.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/__tests__/CallService.test.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/__tests__/PhoneNumberService.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/features/calAIPhone/providers/retellAI/services/tests/PhoneNumberService.test.ts
- packages/features/calAIPhone/providers/retellAI/services/BillingService.ts
- packages/features/calAIPhone/providers/retellAI/RetellSDKClient.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/features/calAIPhone/providers/retellAI/services/__tests__/BillingService.test.tspackages/features/calAIPhone/providers/retellAI/services/CallService.tspackages/features/calAIPhone/providers/retellAI/services/__tests__/CallService.test.tspackages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.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/calAIPhone/providers/retellAI/services/__tests__/BillingService.test.tspackages/features/calAIPhone/providers/retellAI/services/CallService.tspackages/features/calAIPhone/providers/retellAI/services/__tests__/CallService.test.tspackages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
**/*Service.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/features/calAIPhone/providers/retellAI/services/CallService.tspackages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.870Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.665Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
📚 Learning: 2025-08-08T09:29:11.665Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.665Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/__tests__/BillingService.test.tspackages/features/calAIPhone/providers/retellAI/services/CallService.tspackages/features/calAIPhone/providers/retellAI/services/__tests__/CallService.test.tspackages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
📚 Learning: 2025-08-05T13:17:23.491Z
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/__tests__/BillingService.test.tspackages/features/calAIPhone/providers/retellAI/services/CallService.tspackages/features/calAIPhone/providers/retellAI/services/__tests__/CallService.test.tspackages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
📚 Learning: 2025-08-08T09:27:23.870Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.870Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/__tests__/BillingService.test.tspackages/features/calAIPhone/providers/retellAI/services/CallService.tspackages/features/calAIPhone/providers/retellAI/services/__tests__/CallService.test.tspackages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
📚 Learning: 2025-08-05T12:04:29.037Z
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/CallService.tspackages/features/calAIPhone/providers/retellAI/services/__tests__/CallService.test.ts
📚 Learning: 2025-08-08T09:12:08.269Z
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.269Z
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/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : Ensure the `credential.key` field is never returned from tRPC endpoints or APIs
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Repository.ts : Repository files must include `Repository` suffix, prefix with technology if applicable (e.g., `PrismaAppRepository.ts`), and use PascalCase matching the exported class
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/AgentService.ts
🧬 Code Graph Analysis (1)
packages/features/calAIPhone/providers/retellAI/services/AgentService.ts (6)
packages/features/calAIPhone/providers/retellAI/types.ts (3)
RetellAIRepository(143-165)Language(44-80)getLlmId(37-42)packages/features/calAIPhone/providers/interfaces/AgentRepositoryInterface.ts (1)
AgentRepositoryInterface(6-72)packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (4)
AIPhoneServiceAgent(56-57)AIPhoneServiceTools(81-82)AIPhoneServiceUpdateModelParams(49-51)AIPhoneServiceModel(53-54)apps/api/v2/src/lib/logger.bridge.ts (1)
error(77-79)packages/features/calAIPhone/providers/retellAI/RetellAIServiceMapper.ts (1)
RetellAIServiceMapper(24-209)packages/features/calAIPhone/providers/retellAI/services/AIConfigurationService.ts (3)
setupAIConfiguration(22-67)updateLLMConfiguration(141-160)deleteAIConfiguration(69-139)
⏰ Context from checks skipped due to timeout of 90000ms. 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 (13)
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts (1)
17-25: Naming and structure match project conventionsService suffix, PascalCase class, DI through constructor: consistent with guidelines.
packages/features/calAIPhone/providers/retellAI/services/AgentService.ts (1)
18-24: Structure and naming align with conventionsService suffix and DI are consistent; logger scoping is correct.
packages/features/calAIPhone/providers/retellAI/services/__tests__/CallService.test.ts (4)
45-85: Solid coverage for createPhoneCall pathsHappy path, dynamic variables, and external failure are well covered.
122-136: Team-based credit validation scenario coveredTests assert team metadata flows through and behavior remains consistent—good.
138-150: Credit validation edge cases comprehensively testedInsufficient credits, error from credit service, and null credit values are all verified.
Also applies to: 235-250, 252-267
190-202: Rate limiting behavior validated per-userCorrect identifier formation and error propagation are asserted—nice.
Also applies to: 204-219
packages/features/calAIPhone/providers/retellAI/services/__tests__/BillingService.test.ts (4)
64-86: Checkout session creation and metadata assertions look correctURLs, mode, and metadata (userId/workflow/type) are properly asserted.
88-106: Team billing metadata path coveredIncludes teamId and agentId in metadata—good coverage.
145-171: Successful cancellation flow thoroughly validatedStripe cancellation, DB status update with disconnectOutboundAgent, and Retell deletion are asserted.
212-224: Robust negative path coverage for cancellationPhone not found, no active subscription, Stripe failure, Retell deletion failure, and DB update failure are all tested.
Also applies to: 226-241, 243-256
packages/features/calAIPhone/providers/retellAI/services/CallService.ts (3)
70-91: Test-call validations are clear and consistentAgent ID, credits, rate limiting, destination number, agent existence, and assigned phone number are all validated with HttpError and structured logging.
Also applies to: 98-112, 114-124
126-160: Credit validation flow and error semantics look good
- Uses dynamic import to avoid hard coupling.
- Returns 403 when insufficient credits; 500 on service errors; logs context.
14-20: Conventions: Service suffix and constructor DI are consistentLogger namespacing is aligned with other services.
| this.logger.error("Failed to get agent from external AI service", { | ||
| agentId, | ||
| error, | ||
| }); | ||
| throw new HttpError({ | ||
| statusCode: 500, | ||
| message: `Failed to get agent ${agentId}`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Redact agent ID from client-facing error message
Keep identifiers in logs; return a generic message to clients.
} catch (error) {
this.logger.error("Failed to get agent from external AI service", {
agentId,
error,
});
throw new HttpError({
statusCode: 500,
- message: `Failed to get agent ${agentId}`,
+ message: "Failed to get agent",
});
}📝 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.
| this.logger.error("Failed to get agent from external AI service", { | |
| agentId, | |
| error, | |
| }); | |
| throw new HttpError({ | |
| statusCode: 500, | |
| message: `Failed to get agent ${agentId}`, | |
| }); | |
| } | |
| } catch (error) { | |
| this.logger.error("Failed to get agent from external AI service", { | |
| agentId, | |
| error, | |
| }); | |
| throw new HttpError({ | |
| statusCode: 500, | |
| message: "Failed to get agent", | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/features/calAIPhone/providers/retellAI/services/AgentService.ts
around lines 37 to 45, the error message thrown to the client includes the agent
ID, which should be redacted for security. Modify the thrown HttpError to return
a generic error message without the agent ID, while keeping the agent ID in the
internal logger.error call for debugging purposes.
| if (!agentId?.trim()) { | ||
| throw new Error("Agent ID is required and cannot be empty"); | ||
| } | ||
|
|
||
| if (!data || Object.keys(data).length === 0) { | ||
| throw new Error("Update data is required"); | ||
| } | ||
|
|
||
| try { | ||
| const updateRequest = RetellAIServiceMapper.mapToUpdateAgentRequest(data); | ||
| return await this.retellRepository.updateAgent(agentId, updateRequest); | ||
| } catch (error) { | ||
| this.logger.error("Failed to update agent in external AI service", { | ||
| agentId, | ||
| data, | ||
| error, | ||
| }); | ||
| throw new Error(`Failed to update agent ${agentId}`); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Standardize to HttpError for validation and failure paths
Use HttpError to align with the rest of the service layer. Also avoid echoing IDs in thrown messages.
): Promise<AIPhoneServiceAgent<AIPhoneServiceProviderType.RETELL_AI>> {
if (!agentId?.trim()) {
- throw new Error("Agent ID is required and cannot be empty");
+ throw new HttpError({ statusCode: 400, message: "Agent ID is required and cannot be empty" });
}
if (!data || Object.keys(data).length === 0) {
- throw new Error("Update data is required");
+ throw new HttpError({ statusCode: 400, message: "Update data is required" });
}
try {
const updateRequest = RetellAIServiceMapper.mapToUpdateAgentRequest(data);
return await this.retellRepository.updateAgent(agentId, updateRequest);
} catch (error) {
this.logger.error("Failed to update agent in external AI service", {
agentId,
data,
error,
});
- throw new Error(`Failed to update agent ${agentId}`);
+ throw new HttpError({ statusCode: 500, message: "Failed to update agent" });
}
}📝 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 (!agentId?.trim()) { | |
| throw new Error("Agent ID is required and cannot be empty"); | |
| } | |
| if (!data || Object.keys(data).length === 0) { | |
| throw new Error("Update data is required"); | |
| } | |
| try { | |
| const updateRequest = RetellAIServiceMapper.mapToUpdateAgentRequest(data); | |
| return await this.retellRepository.updateAgent(agentId, updateRequest); | |
| } catch (error) { | |
| this.logger.error("Failed to update agent in external AI service", { | |
| agentId, | |
| data, | |
| error, | |
| }); | |
| throw new Error(`Failed to update agent ${agentId}`); | |
| } | |
| if (!agentId?.trim()) { | |
| throw new HttpError({ statusCode: 400, message: "Agent ID is required and cannot be empty" }); | |
| } | |
| if (!data || Object.keys(data).length === 0) { | |
| throw new HttpError({ statusCode: 400, message: "Update data is required" }); | |
| } | |
| try { | |
| const updateRequest = RetellAIServiceMapper.mapToUpdateAgentRequest(data); | |
| return await this.retellRepository.updateAgent(agentId, updateRequest); | |
| } catch (error) { | |
| this.logger.error("Failed to update agent in external AI service", { | |
| agentId, | |
| data, | |
| error, | |
| }); | |
| throw new HttpError({ statusCode: 500, message: "Failed to update agent" }); | |
| } |
🤖 Prompt for AI Agents
In packages/features/calAIPhone/providers/retellAI/services/AgentService.ts
around lines 58 to 76, replace the generic Error throws for validation failures
with HttpError instances to standardize error handling across the service layer.
For validation errors, throw HttpError with appropriate status codes (e.g., 400
Bad Request) and clear messages without echoing agentId. For the catch block,
log the error as is but throw a generic HttpError with a suitable status code
(e.g., 500 Internal Server Error) and a message that does not include the
agentId to avoid exposing sensitive information.
| this.logger.error("Failed to get phone number from external AI service", { | ||
| phoneNumber, | ||
| error, | ||
| }); | ||
| throw new HttpError({ | ||
| statusCode: 500, | ||
| message: `Failed to get phone number '${phoneNumber}'`, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid echoing phone numbers in client-facing error messages
Keep the detailed phone number in logs, but redact from the thrown error to reduce PII exposure.
} catch (error) {
this.logger.error("Failed to get phone number from external AI service", {
phoneNumber,
error,
});
throw new HttpError({
statusCode: 500,
- message: `Failed to get phone number '${phoneNumber}'`,
+ message: "Failed to get phone number",
});
}📝 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.
| this.logger.error("Failed to get phone number from external AI service", { | |
| phoneNumber, | |
| error, | |
| }); | |
| throw new HttpError({ | |
| statusCode: 500, | |
| message: `Failed to get phone number '${phoneNumber}'`, | |
| }); | |
| } catch (error) { | |
| this.logger.error("Failed to get phone number from external AI service", { | |
| phoneNumber, | |
| error, | |
| }); | |
| throw new HttpError({ | |
| statusCode: 500, | |
| message: "Failed to get phone number", | |
| }); | |
| } |
🤖 Prompt for AI Agents
In
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts
around lines 180 to 187, the thrown HttpError message currently includes the
actual phone number, which risks exposing PII to clients. Modify the error
message to remove or redact the phone number while keeping the detailed phone
number in the logger.error call for internal debugging. This ensures sensitive
data is not exposed in client-facing errors.
packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts
Show resolved
Hide resolved
…ure' into feat/cal-ai-self-serve-architecture
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/features/ee/workflows/lib/getOptions.ts (1)
3-4: Type-only import used as a value (compile-time error).Line 16 references WorkflowActions as a runtime value, but Line 3 imports it with
import type, which erases it at runtime. Import WorkflowActions as a value.Apply this diff:
-import type { WorkflowActions } from "@calcom/prisma/enums"; -import { WorkflowTemplates, WorkflowTriggerEvents } from "@calcom/prisma/enums"; +import { WorkflowActions, WorkflowTemplates, WorkflowTriggerEvents } from "@calcom/prisma/enums";Also applies to: 16-16
🧹 Nitpick comments (2)
packages/features/ee/workflows/lib/getOptions.ts (2)
29-36: Typo and naming consistency:filterdWorkflowTriggerEvents→filteredWorkflowTriggerEvents.Aligns with the contributing guide on clear, correctly spelled identifiers.
- const filterdWorkflowTriggerEvents = WORKFLOW_TRIGGER_EVENTS.filter( + const filteredWorkflowTriggerEvents = WORKFLOW_TRIGGER_EVENTS.filter( @@ - return filterdWorkflowTriggerEvents.map((triggerEvent) => { + return filteredWorkflowTriggerEvents.map((triggerEvent) => {
57-58: Use strict equality.Prefer
===over==to avoid unintended coercion.- needsTeamsUpgrade: !hasPaidPlan && template == WorkflowTemplates.CUSTOM, + needsTeamsUpgrade: !hasPaidPlan && template === WorkflowTemplates.CUSTOM,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/features/ee/workflows/lib/getOptions.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/ee/workflows/lib/getOptions.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/ee/workflows/lib/getOptions.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.870Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.352Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.665Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
📚 Learning: 2025-08-08T09:27:23.870Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.870Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
packages/features/ee/workflows/lib/getOptions.ts
📚 Learning: 2025-08-08T10:26:13.352Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.352Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Applied to files:
packages/features/ee/workflows/lib/getOptions.ts
📚 Learning: 2025-08-08T09:29:11.665Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.665Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/features/ee/workflows/lib/getOptions.ts
🧬 Code Graph Analysis (1)
packages/features/ee/workflows/lib/getOptions.ts (1)
packages/features/ee/workflows/lib/constants.ts (1)
WORKFLOW_ACTIONS(13-22)
There was a problem hiding this comment.
LGTM from the architecture perspective !! Great work with this huge PR @Udit-takkar
* feat: cal ai self serve architecture * chore: add package * chore: update evnet controller * refactor: improvements * chore: rename * chore: type error and naming * chore: just set it to nul * chore: just set it to nul * chore: some more improvements * chore: packate version * fix: API v2 * chore: change name of files * chore: add select * chore: add missing teamId * chore: save progress * refactor: split into multiple services * refactor: make schema provider agonistic * chore: improvements * chore: * chore: remove duplicate files * chore: semicolon * chore: formatting * refactor: logging and error handling * chore: rename variable * refactor: use trpc error * chore: replace with HttpError * chore: remove from option * We need the enum and not just the type --------- Co-authored-by: Hariom Balhara <hariombalhara@gmail.com>
What does this PR do?
Related issue #22973
Part 1 of splitting #21827 into parts
This PR has all the backend, database schema related changes along with unit tests.
Overview
The calAIPhone module provides a comprehensive, provider-agnostic AI phone service system for Cal.com, enabling automated phone calls with AI agents for booking and customer
interactions.
🏗️ Architecture & Design Patterns
├── providers/retellAI/
│ ├── services/
│ │ ├── AgentService.ts # Agent CRUD operations
│ │ ├── PhoneNumberService.ts # Phone number management
│ │ ├── CallService.ts # Call handling
│ │ └── BillingService.ts # Subscription management
│ ├── adapters/ # Repository adapters
│ └── RetellAIService.ts # Main service orchestrator
🔧 Key Features
Provider-Agnostic Design
Dual Phone Number Support
Comprehensive Error Handling
Testing Architecture
📊 Database Schema
-- Provider-agnostic agent storage
Agent {
id: String
name: String
providerAgentId: String -- Generic field for any provider
provider: String -- Required: "retell", "openai", etc.
}
-- Phone number management
CalAiPhoneNumber {
id: Int
phoneNumber: String
provider: String -- Required: "retell", "custom-telephony"
providerPhoneNumberId: String -- Generic provider reference
}
🚀 Usage Examples
The module provides three usage patterns:
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
You can run all the unit tests
npx vitest run packages/features/calAIPhone