feat: support web calls in webhook #23788
Conversation
|
Warning Rate limit exceeded@Udit-takkar has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 37 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe PR updates the Retell AI webhook to support web_call events. It introduces PrismaAgentRepository-based agent resolution via agent_id for web calls, makes phone metadata optional, adds a call_type discriminator, and centralizes credit charging in chargeCreditsForCall with duration-based calculation and externalRef retell:${callId}. handleCallAnalyzed now branches for web vs phone calls (agent lookup vs phone-number lookup), validates duration, determines user/team context, and charges credits accordingly. Logging is augmented with call_id context. Tests are expanded to cover web_call scenarios, agent lookup behaviors, credit calculations, team routing, and edge cases; an Agent type import is added for typing. Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
keithwillcode
added
core
dosubot
bot
added
webhooks
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
apps/web/app/api/webhooks/retell-ai/__tests__/route.test.ts (4)
124-133: Fix test intent: currently not “missing signature”.This test passes a signature; it actually validates a failed verification, not a missing header. Either rename or omit the signature to truly test “missing signature.”
Apply one of the following:
- it("should return 401 when signature is missing", async () => { + it("should return 401 when signature header is missing", async () => { const request = createMockRequest( { event: "call_analyzed", call: { call_id: "test-call-id" } }, - "some-signature" + undefined );Or keep the signature and rename:
- it("should return 401 when signature is missing", async () => { + it("should return 401 when signature verification fails", async () => {
347-392: Strengthen assertion for insufficient credits.Add expectations to ensure we don’t charge when credits are unavailable.
const response = await callPOST(request); expect(response.status).toBe(200); + expect(mockHasAvailableCredits).toHaveBeenCalled(); + expect(mockChargeCredits).not.toHaveBeenCalled();
695-756: Add a guard test for disabled agents (web calls).Route selects agent.enabled; add a test to ensure disabled agents are not charged.
describe("Web Call Tests", () => { @@ it("should process web call with valid agent and charge credits", async () => { @@ }); + + it("should skip charging if agent is disabled", async () => { + vi.mocked(PrismaAgentRepository.findByProviderAgentId).mockResolvedValue({ + ...mockAgent, + enabled: false, + }); + const body: RetellWebhookBody = { + event: "call_analyzed", + call: { + call_id: "web-disabled-agent", + call_type: "web_call", + agent_id: mockAgent.providerAgentId!, + call_status: "ended", + call_cost: { total_duration_seconds: 30 }, + }, + }; + const response = await callPOST(createMockRequest(body, "valid-signature")); + expect(response.status).toBe(200); + expect(mockChargeCredits).not.toHaveBeenCalled(); + });
161-177: Consider adding an inbound phone-call test.We skip inbound calls; add an assertion that no charge occurs and a helpful message is returned.
it("should handle non-call_analyzed events", async () => { @@ }); + + it("should ignore inbound phone calls", async () => { + vi.mocked(Retell.verify).mockReturnValue(true); + const body: RetellWebhookBody = { + event: "call_analyzed", + call: { + call_id: "inbound-call", + from_number: "+123", + direction: "inbound", + call_status: "completed", + start_timestamp: 1, + call_cost: { total_duration_seconds: 60 }, + }, + }; + const res = await callPOST(createMockRequest(body, "valid-signature")); + expect(res.status).toBe(200); + const data = await res.json(); + expect(String(data.message)).toContain("Inbound calls are not charged"); + expect(mockChargeCredits).not.toHaveBeenCalled(); + });apps/web/app/api/webhooks/retell-ai/route.ts (6)
22-27: Schema: limit call_type or infer web calls.If Retell consistently uses “web_call”, narrow the schema to reduce typos, or rely solely on presence of agent_id/absence of from_number to infer web calls.
- call_type: z.string().optional(), + // If Retell only emits "web_call", consider tightening: + // call_type: z.literal("web_call").optional(),If docs show more variants, switch to z.enum([...]).
65-77: Remove unused callCost parameter; simplify logs and call sites.callCost isn’t used in the calculation and only appears in error logs; drop it to avoid confusion.
async function chargeCreditsForCall({ userId, teamId, - callCost, callId, callDuration, }: { userId?: number; teamId?: number; - callCost: number; callId: string; callDuration: number; }) { @@ try { await creditService.chargeCredits({ @@ creditFor: CreditUsageType.CAL_AI_PHONE_CALL, externalRef: `retell:${callId}`, }); return { success: true, message: `Successfully charged ${creditsToDeduct} credits (${callDuration}s at $${safeRatePerMinute}/min) for ${ teamId ? `team:${teamId}` : "" } ${userId ? `user:${userId}` : ""}, call ${callId}`, }; } catch (e) { log.error("Error charging credits for Retell AI call", { error: e, - call_id: callId, - call_cost: callCost, + call_id: callId, userId, teamId, }); return { success: false, message: `Error charging credits for Retell AI call: ${ e instanceof Error ? e.message : "Unknown error" }`, }; } } @@ return await chargeCreditsForCall({ userId, teamId, - callCost: call_cost.combined_cost || 0, callId: call_id, callDuration: call_cost.total_duration_seconds, });Also applies to: 95-105, 106-113, 181-187
122-136: Propagate failures instead of returning undefined; add typing for callData.Early returns currently report success=true to the client. Return a result with success=false and type the input to prevent regressions.
-async function handleCallAnalyzed(callData: any) { +async function handleCallAnalyzed(callData: z.infer<typeof RetellWebhookSchema>["call"]) { const { from_number, call_id, call_cost, call_type, agent_id } = callData; if ( !call_cost || typeof call_cost.total_duration_seconds !== "number" || !Number.isFinite(call_cost.total_duration_seconds) || call_cost.total_duration_seconds <= 0 ) { - log.error( - `Invalid or missing call_cost.total_duration_seconds for call ${call_id}: ${safeStringify(call_cost)}` - ); - return; + const msg = `Invalid or missing call_cost.total_duration_seconds for call ${call_id}: ${safeStringify(call_cost)}`; + log.error(msg); + return { success: false, message: msg }; } @@ if (call_type === "web_call" || !from_number) { if (!agent_id) { - log.error(`Web call ${call_id} missing agent_id, cannot charge credits`); - return; + const msg = `Web call ${call_id} missing agent_id, cannot charge credits`; + log.error(msg); + return { success: false, message: msg }; } const agent = await PrismaAgentRepository.findByProviderAgentId({ providerAgentId: agent_id, }); if (!agent) { - log.error(`No agent found for providerAgentId ${agent_id}, call ${call_id}`); - return; + const msg = `No agent found for providerAgentId ${agent_id}, call ${call_id}`; + log.error(msg); + return { success: false, message: msg }; } + if (!agent.enabled) { + const msg = `Agent ${agent_id} is disabled; skipping charge for call ${call_id}`; + log.warn(msg); + return { success: false, message: msg }; + } + userId = agent.userId; teamId = agent.teamId ?? undefined; log.info(`Processing web call ${call_id} for agent ${agent_id}, user ${userId}, team ${teamId}`); } else { const phoneNumber = await PrismaPhoneNumberRepository.findByPhoneNumber({ phoneNumber: from_number, }); if (!phoneNumber) { - log.error(`No phone number found for ${from_number}, call ${call_id}`); - return; + const msg = `No phone number found for ${from_number}, call ${call_id}`; + log.error(msg); + return { success: false, message: msg }; } userId = phoneNumber.userId ?? undefined; teamId = phoneNumber.teamId ?? undefined; log.info(`Processing phone call ${call_id} from ${from_number}, user ${userId}, team ${teamId}`); } if (!userId && !teamId) { - log.error(`Call ${call_id} has no associated user or team`); - return; + const msg = `Call ${call_id} has no associated user or team`; + log.error(msg); + return { success: false, message: msg }; } @@ return NextResponse.json( { - success: result?.success ?? true, + success: result?.success ?? false, message: result?.message ?? `Processed ${payload.event} for call ${callData.call_id}`, }, { status: 200 } );Also applies to: 142-159, 165-179, 265-269
147-159: Authorization guardrails for agent-based charging.Relying solely on providerAgentId risks cross-tenant charging if an unrelated agent_id appears. At minimum, verify agent.enabled (added above). If possible, also verify provider/source and that the agent is configured for web calls.
Would you like me to wire a repository method that also selects provider/type and enforce a provider check here?
95-97: CreditUsageType naming for web calls.Currently we mark all charges as CAL_AI_PHONE_CALL, including web calls. If a dedicated enum exists (e.g., CAL_AI_WEB_CALL), consider passing it for cleaner reporting/analytics.
272-281: Structured error logging.Prefer structured logging over stringifying error as a positional arg.
- log.error("Error processing Retell AI webhook:", safeStringify(error)); + log.error("Error processing Retell AI webhook", { error });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/app/api/webhooks/retell-ai/__tests__/route.test.ts(4 hunks)apps/web/app/api/webhooks/retell-ai/route.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/web/app/api/webhooks/retell-ai/__tests__/route.test.tsapps/web/app/api/webhooks/retell-ai/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/web/app/api/webhooks/retell-ai/__tests__/route.test.tsapps/web/app/api/webhooks/retell-ai/route.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/app/api/webhooks/retell-ai/__tests__/route.test.tsapps/web/app/api/webhooks/retell-ai/route.ts
🧠 Learnings (1)
📚 Learning: 2025-08-27T12:15:43.830Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/testCall.handler.ts:41-44
Timestamp: 2025-08-27T12:15:43.830Z
Learning: In calcom/cal.com, the AgentService.getAgent() method in packages/features/calAIPhone/providers/retellAI/services/AgentService.ts does NOT include authorization checks - it only validates the agentId parameter and directly calls the repository without verifying user/team access. This contrasts with other methods like getAgentWithDetails() which properly use findByIdWithUserAccessAndDetails() for authorization. When reviewing updateToolsFromAgentId() calls, always verify both agent ownership and eventType ownership are checked.
Applied to files:
apps/web/app/api/webhooks/retell-ai/__tests__/route.test.tsapps/web/app/api/webhooks/retell-ai/route.ts
🧬 Code graph analysis (2)
apps/web/app/api/webhooks/retell-ai/__tests__/route.test.ts (2)
packages/lib/server/repository/PrismaAgentRepository.ts (1)
PrismaAgentRepository(31-573)packages/lib/server/repository/PrismaPhoneNumberRepository.ts (1)
PrismaPhoneNumberRepository(26-535)
apps/web/app/api/webhooks/retell-ai/route.ts (2)
packages/lib/server/repository/PrismaAgentRepository.ts (1)
PrismaAgentRepository(31-573)packages/lib/server/repository/PrismaPhoneNumberRepository.ts (1)
PrismaPhoneNumberRepository(26-535)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
- GitHub Check: Linters / lint
🔇 Additional comments (4)
apps/web/app/api/webhooks/retell-ai/__tests__/route.test.ts (4)
5-8: Good: types tightened for mocks.Importing Agent and using precise Pick typing for mockAgent improves test clarity.
21-31: Schema parity: exercised new optional fields.Tests reflect optional phone fields and added call_type; this aligns with the route schema.
90-95: Repository mock wired correctly.Mocking PrismaAgentRepository.findByProviderAgentId enables deterministic web-call tests.
523-577: Idempotency coverage looks solid.Verifies externalRef stability and duplicate handling; no action needed.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/api/webhooks/retell-ai/route.ts (1)
87-97: Guard against ambiguous owner (both userId and teamId)Ensure exactly one owner is charged to prevent misattribution.
const creditService = new CreditService(); + // Exactly one owner must be provided + const owners = Number(Boolean(userId)) + Number(Boolean(teamId)); + if (owners !== 1) { + return { + success: false, + message: `Ambiguous owner: expected exactly one of userId or teamId (userId=${userId}, teamId=${teamId}).`, + }; + } + try { await creditService.chargeCredits({ userId: userId ?? undefined, teamId: teamId ?? undefined, credits: creditsToDeduct, callDuration: callDuration, creditFor: CreditUsageType.CAL_AI_PHONE_CALL, externalRef: `retell:${callId}`, });
🧹 Nitpick comments (5)
apps/web/app/api/webhooks/retell-ai/route.ts (5)
22-27: Constrain call_type and tighten validation for web vs phone callsUse an enum for call_type to avoid typos, and prefer schema-level validation to ensure web calls have agent_id and phone calls have from_number.
- call_type: z.string().optional(), + call_type: z.enum(["web_call", "phone_call"]).optional(),Optionally, add a cross-field refinement to catch missing prerequisites early:
- .passthrough(), + .passthrough() + .superRefine((val, ctx) => { + const c = val.call ?? {}; + if ((c.call_type === "web_call" || !c.from_number) && !c.agent_id) { + ctx.addIssue({ code: z.ZodIssueCode.custom, message: "agent_id is required for web calls" }); + } + if (c.call_type !== "web_call" && !c.from_number) { + ctx.addIssue({ code: z.ZodIssueCode.custom, message: "from_number is required for phone calls" }); + } + }),
65-77: Remove unused callCost parameter to avoid confusioncallCost is not used in chargeCreditsForCall; drop it from the signature and callers.
-async function chargeCreditsForCall({ - userId, - teamId, - callCost, - callId, - callDuration, -}: { +async function chargeCreditsForCall({ + userId, + teamId, + callId, + callDuration, +}: { userId?: number; teamId?: number; - callCost: number; callId: string; callDuration: number; }) {
106-112: Improve error context; avoid logging unused callCostLog actionable fields that explain computed charge.
- log.error("Error charging credits for Retell AI call", { - error: e, - call_id: callId, - call_cost: callCost, - userId, - teamId, - }); + log.error("Error charging credits for Retell AI call", { + error: e, + call_id: callId, + userId, + teamId, + callDuration, + creditsToDeduct, + ratePerMinute: safeRatePerMinute, + });
184-190: Update chargeCreditsForCall invocation after signature changeRemove unused callCost argument.
return await chargeCreditsForCall({ userId, teamId, - callCost: call_cost.combined_cost || 0, callId: call_id, callDuration: call_cost.total_duration_seconds, });
147-154: Ownership/authorization check on agent lookupPrismaAgentRepository.findByProviderAgentId returns the agent without access checks. Since this endpoint charges credits, verify that this providerAgentId legitimately maps to the tenant expected for this Retell account, and consider asserting ownership constraints if available.
Would you like me to wire a repository method that enforces “exactly one of userId/teamId” and “enabled agent” invariants?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/web/app/api/webhooks/retell-ai/route.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/web/app/api/webhooks/retell-ai/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/web/app/api/webhooks/retell-ai/route.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/app/api/webhooks/retell-ai/route.ts
🧠 Learnings (1)
📚 Learning: 2025-08-27T12:15:43.830Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/testCall.handler.ts:41-44
Timestamp: 2025-08-27T12:15:43.830Z
Learning: In calcom/cal.com, the AgentService.getAgent() method in packages/features/calAIPhone/providers/retellAI/services/AgentService.ts does NOT include authorization checks - it only validates the agentId parameter and directly calls the repository without verifying user/team access. This contrasts with other methods like getAgentWithDetails() which properly use findByIdWithUserAccessAndDetails() for authorization. When reviewing updateToolsFromAgentId() calls, always verify both agent ownership and eventType ownership are checked.
Applied to files:
apps/web/app/api/webhooks/retell-ai/route.ts
🧬 Code graph analysis (1)
apps/web/app/api/webhooks/retell-ai/route.ts (2)
packages/lib/server/repository/PrismaAgentRepository.ts (1)
PrismaAgentRepository(31-573)packages/lib/server/repository/PrismaPhoneNumberRepository.ts (1)
PrismaPhoneNumberRepository(26-535)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (2)
apps/web/app/api/webhooks/retell-ai/route.ts (2)
89-97: Idempotency confirmed — externalRef is enforced and tested. schema.prisma declares externalRef String? @unique, and tests in packages/features/ee/billing/credit-service.test.ts and apps/web/app/api/webhooks/retell-ai/tests/route.test.ts verify duplicate externalRef prevents double-charging.
95-96: Add a web-call CreditUsageType or ensure callDuration is set when logging Retell calls.Prisma only defines 'CAL_AI_PHONE_CALL' (packages/prisma/schema.prisma) and the migration adds a DB CHECK that requires callDuration for CAL_AI_PHONE_CALL (packages/prisma/migrations/20250821141229_add_external_ref_and_duration/migration.sql), so either add CAL_AI_WEB_CALL (or generic CAL_AI_CALL) + migration, or include callDuration when writing CreditUsageType.CAL_AI_PHONE_CALL in apps/web/app/api/webhooks/retell-ai/route.ts (lines 95–96).
E2E results are ready! |
What does this PR do?
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?