Conversation
WalkthroughThis update introduces a Twilio-integrated AI voice call system using ElevenLabs conversational AI, with new WebSocket and HTTP routes for handling calls, voice webhooks, and AI agent interactions. It adds new database schema for JWKS, updates authentication and session context handling, and expands the API with voice endpoints and supporting services. Changes
Sequence Diagram(s)sequenceDiagram
participant Twilio as Twilio Voice Webhook
participant Server as API Server
participant ElevenLabs as ElevenLabs API
participant MCP as ZeroMCP Service
participant User as User
Twilio->>Server: POST /api/ai/voice (voice webhook)
Server->>Twilio: Respond with TwiML to connect to /api/ai/call/:callSid (WebSocket)
Twilio->>Server: WebSocket upgrade /api/ai/call/:callSid
Server->>Server: CallService.startCall()
Server->>ElevenLabs: Connect via signed URL (WebSocket)
Server->>MCP: Map phone to user, get driver
ElevenLabs->>Server: AI agent messages (WebSocket)
Server->>Twilio: Forward audio/messages
Server->>MCP: Tool calls (e.g., manage_email)
MCP->>Server: Tool results
Server->>ElevenLabs: Send tool results
ElevenLabs->>Server: Audio/messages
Server->>Twilio: Send audio/messages
User->>Twilio: Speaks on call
Twilio->>Server: Media events (WebSocket)
Server->>ElevenLabs: Forward audio
Possibly related PRs
Suggested reviewers
Poem
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (5)
apps/server/package.json (1)
1-82:⚠️ Potential issueCritical: Fix the outdated pnpm-lock.yaml file.
The pipeline failure indicates that the
pnpm-lock.yamlfile is not synchronized with the package.json changes. This needs to be resolved immediately.Run the following to fix the lockfile:
pnpm install --no-frozen-lockfileThen commit the updated
pnpm-lock.yamlfile to resolve the CI failure.🧰 Tools
🪛 GitHub Actions: autofix.ci
[error] 1-1: pnpm install failed due to outdated pnpm-lock.yaml. The lockfile is not up to date with package.json dependencies. Use 'pnpm install --no-frozen-lockfile' to bypass this check.
apps/server/src/trpc/routes/connections.ts (2)
18-31:⚠️ Potential issueAdd null safety check for sessionUser.
While line 81 checks
if (!ctx.sessionUser), this procedure destructures and usessessionUser.iddirectly without null checking. This could cause runtime errors if sessionUser is undefined.Apply this diff to add proper null checking:
.query(async ({ ctx }) => { - const { db, sessionUser } = ctx; + const { db, sessionUser } = ctx; + if (!sessionUser) { + throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' }); + } const connections = await db
56-64:⚠️ Potential issueAdd null safety check for sessionUser.
Similar to the list procedure, this mutation uses
sessionUserwithout null checking.Apply this diff:
.mutation(async ({ input, ctx }) => { const { connectionId } = input; const { db } = ctx; - const user = ctx.sessionUser; + const user = ctx.sessionUser; + if (!user) { + throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' }); + }package.json (1)
1-45: 💡 Verification agent🧩 Analysis chain
Address the pipeline failure: Update pnpm lockfile.
The pipeline is failing because the pnpm lockfile is outdated. Run
pnpm installto update the lockfile and resolve this issue.
🏁 Script executed:
#!/bin/bash # Check if pnpm-lock.yaml exists and verify its status echo "Checking pnpm lockfile status..." if [ -f "pnpm-lock.yaml" ]; then echo "pnpm-lock.yaml exists" # Check if dependencies in package.json match lockfile expectations echo "To fix the issue, run: pnpm install" else echo "pnpm-lock.yaml not found" fiLength of output: 236
Update pnpm lockfile
Run
pnpm installto regenerate and commit the updatedpnpm-lock.yamlto resolve the CI/pipeline failure.• Ensure the root pnpm-lock.yaml is updated and included in your commit.
🧰 Tools
🪛 GitHub Actions: autofix.ci
[error] 1-1: pnpm install failed due to outdated pnpm-lock.yaml. The lockfile is not up to date with package.json dependencies. Use 'pnpm install --no-frozen-lockfile' to bypass this check.
apps/server/src/routes/agent/tools.ts (1)
273-273:⚠️ Potential issueFix typo in description.
- description: 'Create a new label with custom colors, if it does nto exist already', + description: 'Create a new label with custom colors, if it does not exist already',
🧹 Nitpick comments (19)
apps/server/src/services/call-service/eleven-labs-outgoing-message-schema.ts (1)
1-63: Well-structured schema with comprehensive message type coverage.The ElevenLabs outgoing message schema is well-designed with good separation of concerns. The union type approach provides type safety while supporting multiple message formats.
However, consider these minor improvements:
- Add JSDoc documentation for better developer experience:
+/** + * Schema for validating outgoing messages to ElevenLabs WebSocket + * Supports various message types including audio chunks, conversation config, and tool results + */ export const elevenLabsOutgoingSocketMessageSchema = z.union([
- Consider extracting common schemas for reusability:
+const baseConversationConfig = z.object({ + agent: z.object({ + prompt: z.object({ + prompt: z.string().nullable().default(null), + }).optional(), + first_message: z.string().optional(), + language: z.string().optional(), + }).optional(), + tts: z.object({ + voice_id: z.string(), + }).optional(), +}); // Then use in the conversation_initiation_client_data message
- Add runtime validation helpers:
export const validateElevenLabsMessage = (data: unknown): ElevenLabsOutgoingSocketMessage => { return elevenLabsOutgoingSocketMessageSchema.parse(data); };apps/server/src/trpc/routes/ai/search.ts (1)
12-12: LGTM: Context parameter added for consistency.The addition of the
ctxparameter aligns with the broader authentication context standardization across the codebase. While currently unused, this provides consistency with other procedures and enables future access to session context.Consider adding a brief comment if the
ctxparameter is intended for future use:- .mutation(async ({ input, ctx }) => { + .mutation(async ({ input, ctx }) => { // ctx reserved for future session context usageapps/server/src/trpc/trpc.ts (1)
28-28: Consider simplifying the context spreading.The context spreading
{ ...ctx, sessionUser: ctx.sessionUser }appears redundant sincesessionUseris already in the context. This could be simplified to just passctx.- return next({ ctx: { ...ctx, sessionUser: ctx.sessionUser } }); + return next({ ctx });apps/server/src/trpc/routes/voice/get-signed-url.ts (1)
9-18: Consider adding error handling for ElevenLabs API calls.The route makes an external API call without explicit error handling. Consider wrapping the ElevenLabs call in a try-catch block to provide more meaningful error messages.
export const getSignedUrl = activeConnectionProcedure.query(async ({ ctx }) => { - const response = await elevenLabs.conversationalAi.getSignedUrl({ - agent_id: env.ELEVENLABS_AGENT_ID, - }); + try { + const response = await elevenLabs.conversationalAi.getSignedUrl({ + agent_id: env.ELEVENLABS_AGENT_ID, + }); + } catch (error) { + throw new TRPCError({ + code: 'INTERNAL_SERVER_ERROR', + message: 'Failed to get signed URL from ElevenLabs', + cause: error, + }); + }apps/server/src/lib/auth.ts (2)
302-332: Consider using cryptographically secure token generation.The helper function is well-implemented with proper session reuse logic. However, consider using a more cryptographically secure method for token generation instead of
crypto.randomUUID().- const newToken = crypto.randomUUID(); + const newToken = crypto.randomUUID() + '-' + crypto.randomUUID(); // Enhanced entropyOr better yet, use a dedicated token generation library that provides cryptographically secure tokens for authentication purposes.
306-309: Consider adding database index for performance.The query filters by
userIdandexpiresAtwhich could benefit from a compound index for better performance, especially as the session table grows.-- Consider adding this index for better query performance CREATE INDEX idx_session_user_expires ON session(userId, expiresAt DESC);apps/server/src/services/call-service/twilio-socket-message-schema.ts (2)
18-21: Consider adding validation constraints for media format.The media format fields could benefit from more specific validation to ensure they match expected Twilio values.
mediaFormat: z.object({ - encoding: z.string(), - sampleRate: z.number(), - channels: z.number(), + encoding: z.enum(['audio/x-mulaw', 'audio/x-l16']), + sampleRate: z.number().int().positive(), + channels: z.number().int().min(1).max(2), }),
32-33: Consider base64 validation for media payload.Since Twilio sends audio data as base64-encoded strings, consider adding base64 format validation to the payload field.
media: z.object({ - payload: z.string(), + payload: z.string().regex(/^[A-Za-z0-9+/]*={0,2}$/, 'Invalid base64 format'), }),apps/server/src/main.ts (1)
102-129: Consider removing or documenting the commented-out routes.Large blocks of commented code can reduce maintainability. If these routes are temporarily disabled, consider:
- Adding a comment explaining why they're disabled and when they might be re-enabled
- Moving them to a separate file or branch if they're not needed soon
- Creating a feature flag to enable/disable them dynamically
apps/server/src/services/call-service/eleven-labs-incoming-message-schema.ts (1)
111-111: Add description for client_tool_call.The
.describe()call on line 111 has an empty string. Please add a meaningful description.- .describe(''), + .describe('Client tool call event data'),apps/server/src/services/mcp-service/mcp.ts (1)
162-184: Remove commented-out code to improve maintainability.Large blocks of commented-out code reduce readability and should be removed. If this code might be needed later, consider tracking it in version control history or a separate feature branch.
apps/server/src/routes/ai.ts (2)
10-26: Consider adding rate limiting for the MCP endpoint.The MCP endpoint validates the phone number header but doesn't implement rate limiting. This could be exploited for DoS attacks or resource exhaustion.
Consider implementing rate limiting based on the phone number or IP address to prevent abuse.
72-77: Add error handling for WebSocket events.Consider adding error and close event handlers to properly manage WebSocket lifecycle and log any issues.
// Handle WebSocket events server.addEventListener('open', () => { console.log(`[Twilio] WebSocket connection opened`); }); + + server.addEventListener('error', (error) => { + console.error(`[Twilio] WebSocket error for call ${callSid}:`, error); + }); + + server.addEventListener('close', (event) => { + console.log(`[Twilio] WebSocket connection closed for call ${callSid}. Code: ${event.code}, Reason: ${event.reason}`); + });apps/server/src/services/call-service/call-service.ts (6)
76-77: Remove commented code or provide clear documentation.The commented line suggests an alternative MCP client implementation. Either remove it or add a comment explaining why both implementations exist.
- // private mcpClient: Client | null = null; private mcpClient: Awaited<ReturnType<typeof createMCPClient>> | null = null;
94-95: Remove commented code or add documentation.Similar to the previous comment, this suggests multiple MCP connection methods exist. Consider documenting why both are needed or remove the unused one.
- // this.mcpClient = await this.connectToMCP(hostname); this.mcpClient = await this.connectToMCP_AISDK(hostname);
283-287: Use optional chaining for better readability.The conditional check can be simplified using optional chaining as suggested by the static analysis tool.
if ( - data.client_tool_call && - data.client_tool_call.tool_name && - data.client_tool_call.tool_call_id + data.client_tool_call?.tool_name && + data.client_tool_call?.tool_call_id ) {🧰 Tools
🪛 Biome (1.9.4)
[error] 284-285: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
378-387: Remove commented-out code.Large blocks of commented code should be removed to maintain code cleanliness. If this functionality is needed later, it can be retrieved from version control.
- // get driver and connection id - // switch (toolName) { - // case 'get_thread': - // break; - // case 'list_threads': - // break; - // default: - // console.warn('[ElevenLabs] Unhandled tool call:', toolName); - // } }
447-476: Remove large commented code blocks.This commented method should be removed entirely. Large commented code blocks reduce readability and should be managed through version control instead.
Remove the entire commented
getMcpToolSetmethod (lines 447-476).
478-500: Consider removing unused MCP connection method.Since the service uses
connectToMCP_AISDK, this method appears unused. Consider removing it or documenting why both implementations are needed.If this method is not used, remove it to reduce code complexity. If it's kept as an alternative implementation, add documentation explaining the difference.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
apps/server/package.json(2 hunks)apps/server/src/ctx.ts(1 hunks)apps/server/src/db/schema.ts(1 hunks)apps/server/src/lib/auth.ts(3 hunks)apps/server/src/lib/prompts.ts(1 hunks)apps/server/src/lib/schemas.ts(0 hunks)apps/server/src/lib/server-utils.ts(1 hunks)apps/server/src/main.ts(3 hunks)apps/server/src/routes/agent/tools.ts(6 hunks)apps/server/src/routes/ai.ts(1 hunks)apps/server/src/services/call-service/call-service.ts(1 hunks)apps/server/src/services/call-service/eleven-labs-incoming-message-schema.ts(1 hunks)apps/server/src/services/call-service/eleven-labs-outgoing-message-schema.ts(1 hunks)apps/server/src/services/call-service/system-prompt.ts(1 hunks)apps/server/src/services/call-service/twilio-socket-message-schema.ts(1 hunks)apps/server/src/services/mcp-service/mcp.ts(1 hunks)apps/server/src/trpc/index.ts(3 hunks)apps/server/src/trpc/routes/ai/index.ts(1 hunks)apps/server/src/trpc/routes/ai/search.ts(1 hunks)apps/server/src/trpc/routes/ai/webSearch.ts(0 hunks)apps/server/src/trpc/routes/connections.ts(5 hunks)apps/server/src/trpc/routes/user.ts(0 hunks)apps/server/src/trpc/routes/voice/get-signed-url.ts(1 hunks)apps/server/src/trpc/routes/voice/index.ts(1 hunks)apps/server/src/trpc/trpc.ts(1 hunks)apps/server/wrangler.jsonc(0 hunks)package.json(2 hunks)
💤 Files with no reviewable changes (4)
- apps/server/wrangler.jsonc
- apps/server/src/lib/schemas.ts
- apps/server/src/trpc/routes/user.ts
- apps/server/src/trpc/routes/ai/webSearch.ts
🧰 Additional context used
🧬 Code Graph Analysis (8)
apps/server/src/trpc/routes/voice/get-signed-url.ts (1)
apps/server/src/trpc/trpc.ts (1)
activeConnectionProcedure(31-42)
apps/server/src/trpc/routes/connections.ts (1)
apps/server/src/db/schema.ts (2)
connection(87-106)user(15-27)
apps/server/src/lib/server-utils.ts (1)
apps/server/src/db/schema.ts (2)
user(15-27)connection(87-106)
apps/server/src/trpc/routes/ai/index.ts (2)
apps/server/src/trpc/routes/ai/search.ts (1)
generateSearchQuery(10-23)apps/server/src/trpc/routes/ai/compose.ts (2)
compose(108-139)generateEmailSubject(141-160)
apps/server/src/trpc/index.ts (1)
apps/server/src/trpc/routes/voice/index.ts (1)
voiceRouter(4-6)
apps/server/src/trpc/routes/voice/index.ts (1)
apps/server/src/trpc/trpc.ts (1)
router(18-18)
apps/server/src/main.ts (4)
apps/server/src/db/schema.ts (1)
user(15-27)apps/server/src/routes/ai.ts (1)
aiRouter(6-6)apps/server/src/trpc/routes/ai/index.ts (1)
aiRouter(5-9)apps/server/src/trpc/index.ts (1)
appRouter(18-31)
apps/server/src/ctx.ts (1)
apps/server/src/lib/auth.ts (1)
Auth(293-293)
🪛 GitHub Actions: autofix.ci
apps/server/package.json
[error] 1-1: pnpm install failed due to outdated pnpm-lock.yaml. The lockfile is not up to date with package.json dependencies. Use 'pnpm install --no-frozen-lockfile' to bypass this check.
package.json
[error] 1-1: pnpm install failed due to outdated pnpm-lock.yaml. The lockfile is not up to date with package.json dependencies. Use 'pnpm install --no-frozen-lockfile' to bypass this check.
🪛 Biome (1.9.4)
apps/server/src/services/mcp-service/mcp.ts
[error] 55-55: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
apps/server/src/services/call-service/call-service.ts
[error] 192-227: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 284-285: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (27)
apps/server/package.json (3)
54-54:Details
✅ Verification successful
Verify jose package version.
The
josepackage is a well-established JWT library. Let's verify this is the latest version.
🌐 Web query:
What is the latest version of the jose npm package and are there any security advisories for version 6.0.11?💡 Result:
Latest Version of the jose npm Package
The latest version of the
josenpm package is 6.0.11, published on May 5, 2025[1][3][5]. This version is officially listed as the most current on both npm and the project's GitHub repository.Security Advisories for Version 6.0.11
There are no known security advisories or direct vulnerabilities reported for version 6.0.11 of the
josepackage according to multiple respected vulnerability databases, including Snyk[4][10]. The Snyk listing explicitly shows that version 6.0.11 does not have any recorded vulnerabilities as of its latest scan.To summarize:
- Current version: 6.0.11
- Security status: No reported vulnerabilities or advisories for 6.0.11[4][10].
If you are using version 6.0.11 of the
josepackage, it is considered up-to-date and free of known, publicly disclosed security issues as of June 2025.Citations:
- 1: https://www.npmjs.com/package/jose
- 2: https://deps.dev/npm/jose/1.21.1/compare
- 3: https://classic.yarnpkg.com/en/package/jose
- 4: https://security.snyk.io/package/npm/jose
- 5: https://github.com/panva/jose/blob/main/package.json
- 6: https://thehackernews.com/2025/05/over-70-malicious-npm-and-vs-code.html
- 7: drizzle-team/drizzle-orm#3067
- 8: panva/jose#577
- 9: https://socket.dev/npm/package/jose/versions/6.0.11?page=19
- 10: https://security.snyk.io/package/npm/jose/5.0.0
jose package version confirmed
- Current version in apps/server/package.json: 6.0.11
- Verified as the latest release (published May 5, 2025)
- No known security advisories or vulnerabilities reported for v6.0.11
No changes needed—this dependency is up-to-date and secure.
47-47:Details
✅ Verification successful
Verify ElevenLabs package version and security.
The
elevenlabspackage version 1.59.0 should be verified for being the latest stable version and free from security vulnerabilities.
🌐 Web query:
What is the latest version of the elevenlabs npm package and are there any known security vulnerabilities in version 1.59.0?💡 Result:
The latest version of the elevenlabs npm package is 1.59.0, which was published 19 days ago as of May 15, 2025[1]. This is the most recent version available on npm.
Regarding security vulnerabilities in version 1.59.0, there is no information in the search results indicating any known security vulnerabilities for this specific version. It's worth noting that for a similar package called "elevenlabs-node" (which is different from the official "elevenlabs" package), Snyk's vulnerability database shows no direct vulnerabilities[6]. However, this information is specifically about the "elevenlabs-node" package rather than the official "elevenlabs" package.
The elevenlabs package is the official JavaScript API for ElevenLabs text-to-speech software, providing rich and lifelike voices to creators and developers. It supports multiple models including Eleven Multilingual v2, Eleven Flash v2.5, and Eleven Turbo v2.5, each with different capabilities for language support and latency[1].
If you're planning to use this package, you can install it using npm:
npm install elevenlabsOr with yarn:
yarn add elevenlabsCitations:
- 1: https://www.npmjs.com/package/elevenlabs
- 2: https://github.com/elevenlabs/packages
- 3: https://www.jsdelivr.com/package/npm/@11labs/client
- 4: https://github.com/FelixWaweru/elevenlabs-node
- 5: https://www.nuget.org/packages/ElevenLabs-DotNet
- 6: https://security.snyk.io/package/npm/elevenlabs-node/1.0.1
- 7: https://git.lukegb.com/lukegb/depot/src/commit/a8ba803d23cef8d3ef59bffb97122bbba6de9818/pkgs/top-level/release-python.nix
- 8: https://elevenlabs.io/docs/changelog
- 9: https://git.lukegb.com/lukegb/depot/src/commit/a8ba803d23cef8d3ef59bffb97122bbba6de9818/doc/doc-support
- 10: https://github.com/j3lte/awesome-raycast
ElevenLabs dependency is up-to-date and secure
- apps/server/package.json (line 47):
elevenlabsset to 1.59.0, which is the latest stable release and has no known security vulnerabilities.
66-66:Details
✅ Verification successful
Verify Twilio SDK version.
The Twilio SDK version should be checked for being current and secure.
🌐 Web query:
What is the latest version of the twilio npm package and are there any known security issues with version 5.7.0?💡 Result:
As of June 3, 2025, the latest version of the Twilio Node.js library (
twilio) is 5.5.0, published 6 days ago. (npmjs.com)Version 5.7.0 was released on April 20, 2023. (github.com) There are no known security vulnerabilities associated with this version. The latest version, 5.5.0, also does not have any reported security issues. (socket.dev)
It's generally recommended to use the latest stable version of libraries to benefit from the most recent features and security patches.
Citations:
- 1: https://www.npmjs.com/package/twilio?utm_source=openai
- 2: https://github.com/twilio/twilio-cli/blob/main/CHANGES.md?utm_source=openai
- 3: https://socket.dev/npm/package/twilio?utm_source=openai
🏁 Script executed:
curl -s https://registry.npmjs.org/twilio | jq -r '.["dist-tags"].latest'Length of output: 81
✅ Twilio SDK version is up-to-date and secure.
According to the npm registry,
5.7.0is the current latest release of thetwiliopackage, and there are no known security vulnerabilities for this version. No changes are required.apps/server/src/trpc/trpc.ts (1)
22-22: LGTM! Authorization check updated correctly.The change from
ctx.session?.usertoctx.sessionUseraligns with the session refactoring across the codebase.apps/server/src/trpc/routes/ai/index.ts (1)
5-9: LGTM! Router definition improved for clarity.The change from shorthand notation to explicit key-value pairs makes the router definition more explicit and maintainable. The removal of
webSearchaligns with the AI phone assistant functionality scope.apps/server/src/lib/server-utils.ts (2)
9-10: LGTM! Session refactoring implemented correctly.The change from
session?.usertosessionUsersimplifies the context access and aligns with the broader authentication refactoring across the codebase.
13-34: Excellent consistency in user ID references.All occurrences of
session.user.idhave been properly updated tosessionUser.id, maintaining the same functionality while simplifying the context access pattern.apps/server/src/trpc/routes/voice/index.ts (1)
1-6: Well-structured voice router implementation.The router follows established patterns and provides a clean interface for the voice functionality. The structure is consistent with other routers in the codebase and properly supports the ElevenLabs integration for AI voice calls.
apps/server/src/trpc/routes/connections.ts (1)
81-83: LGTM! Proper null checking implemented.This procedure correctly checks for sessionUser existence before proceeding.
apps/server/src/lib/prompts.ts (1)
262-262: LGTM! Improved default search behavior.Excluding promotional emails by default is a sensible UX improvement that will make search results more relevant while preserving the option to include them when explicitly requested.
apps/server/src/trpc/index.ts (2)
11-11: LGTM! Voice router properly integrated.The voice router is correctly imported and added to the main app router, enabling the new voice functionality.
Also applies to: 30-30
42-42: LGTM! Consistent session refactor.The change from
sessiontosessionUseraligns with the broader refactor pattern seen across other files in this PR.package.json (2)
7-7: LGTM: Convenient development workflow script.The composite
goscript provides a convenient way to start both the database and development environment in sequence.
23-23: LGTM: Completes the Docker database management scripts.The
docker:db:stopscript complements the existing Docker database scripts by providing a clean stop option.apps/server/src/ctx.ts (2)
6-6: Well-designed type extraction for session user.The
SessionUsertype correctly extracts the non-nullable user from the session, providing better type safety and clearer intent.
10-10: Good refactoring: Simplified session context.Changing from
sessionto optionalsessionUsersimplifies context usage and aligns with the authentication refactoring across the codebase.apps/server/src/lib/auth.ts (2)
9-9: LGTM: JWT and bearer token plugin integration.The addition of
jwtandbearerplugins extends authentication capabilities to support stateless authentication alongside session-based auth.
86-87: LGTM: Proper plugin configuration.The JWT and bearer plugins are correctly added to the auth configuration.
apps/server/src/services/call-service/twilio-socket-message-schema.ts (2)
3-48: Well-structured Zod schema with good type safety.The discriminated union approach provides excellent type safety for handling different Twilio WebSocket event types. The schema correctly models the Twilio Media Stream API message format.
50-50: LGTM: Clean type export pattern.The inferred TypeScript type export follows best practices and provides type safety for consumers of this schema.
apps/server/src/routes/agent/tools.ts (1)
127-241: LGTM! Excellent documentation improvements.Adding
.describe()annotations to all schema fields is a best practice that improves API self-documentation and developer experience. The descriptions are clear, accurate, and will be helpful for anyone consuming these tools.apps/server/src/main.ts (1)
65-67: LGTM! Context update aligns with session handling changes.The updated context creation correctly uses
sessionUserinstead ofsession, maintaining consistency with the authentication middleware changes.apps/server/src/services/call-service/eleven-labs-incoming-message-schema.ts (1)
1-141: Well-structured schema with excellent type safety!The discriminated union pattern with proper nullable handling and comprehensive descriptions makes this schema robust and maintainable. The validation constraints (like the VAD score range) add extra safety.
apps/server/src/routes/ai.ts (1)
27-42: Voice webhook handler implementation looks good.The Twilio voice webhook handler correctly processes the incoming call data and returns a properly formatted TwiML response with WebSocket streaming configuration.
apps/server/src/services/call-service/system-prompt.ts (1)
1-262: System prompt is comprehensive and well-structured.The system prompt effectively defines the AI assistant's behavior, available tools, and interaction patterns. The structured format with numbered sections and clear examples will help ensure consistent AI responses.
apps/server/src/services/call-service/call-service.ts (2)
64-82: Excellent class structure and dependency injection.The CallService class is well-structured with clear separation of concerns. The constructor properly initializes the Twilio client, and the class maintains appropriate state for managing call sessions.
432-435: Good use of AI SDK with proper configuration.The AI response generation uses appropriate model selection (gpt-4o-mini) and includes reasonable limits with maxSteps. The integration with MCP tools is well implemented.
| export const jwks = createTable('jwks', { | ||
| id: text('id').primaryKey(), | ||
| publicKey: text('public_key').notNull(), | ||
| privateKey: text('private_key').notNull(), | ||
| createdAt: timestamp('created_at').notNull(), | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Review the security implications of storing private keys in the database.
The new jwks table stores both public and private keys in plain text. This introduces significant security concerns:
- Private key exposure: Storing private keys in the database creates a single point of failure. If the database is compromised, all JWT signing capabilities are exposed.
- Missing encryption: The private keys should be encrypted at rest.
- Access control: Consider if this table needs additional access restrictions.
Consider these security improvements:
export const jwks = createTable('jwks', {
id: text('id').primaryKey(),
publicKey: text('public_key').notNull(),
- privateKey: text('private_key').notNull(),
+ privateKey: text('private_key').notNull(), // Consider encrypting this field
createdAt: timestamp('created_at').notNull(),
+ expiresAt: timestamp('expires_at'), // Add key expiration
+ isActive: boolean('is_active').notNull().default(true), // Allow key rotation
});Let me verify how these keys are being used in the authentication flow:
🏁 Script executed:
#!/bin/bash
# Description: Check how JWKS private keys are used and if there are any security measures in place
# Search for jwks table usage and private key access patterns
rg -A 10 -B 5 "privateKey|private_key" --type ts
# Check for any encryption/decryption related to jwks
rg -A 5 -B 5 "(encrypt|decrypt|cipher)" --type ts
# Look for jwks table operations
ast-grep --pattern 'jwks'Length of output: 4682
Secure Private Key Storage in jwks Table
No encryption/decryption logic was found around the new jwks table. Currently the private_key column stores raw keys in plaintext, introducing a single point of failure if the database is compromised.
Recommendations:
- Encrypt the
private_keyfield at rest (e.g., use PostgreSQL’s pgcrypto or integrate with a KMS/Vault). - Add
expires_atandis_activecolumns to support key rotation. - Apply least-privilege access controls on this table.
- Continue using in-memory JWK sets for public key verification (
createLocalJWKSet(localJwks)inapps/server/src/main.ts) and only load private keys when needed, decrypting them just-in-time.
Locations to update:
apps/server/src/db/schema.ts:169-174- Authentication setup in
apps/server/src/main.tsaroundcreateLocalJWKSet(localJwks)
Suggested schema diff:
export const jwks = createTable('jwks', {
id: text('id').primaryKey(),
publicKey: text('public_key').notNull(),
- privateKey: text('private_key').notNull(),
+ privateKey: text('private_key').notNull(), // must be encrypted at rest or via KMS
createdAt: timestamp('created_at').notNull(),
+ expiresAt: timestamp('expires_at'),
+ isActive: boolean('is_active').notNull().default(true),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const jwks = createTable('jwks', { | |
| id: text('id').primaryKey(), | |
| publicKey: text('public_key').notNull(), | |
| privateKey: text('private_key').notNull(), | |
| createdAt: timestamp('created_at').notNull(), | |
| }); | |
| export const jwks = createTable('jwks', { | |
| id: text('id').primaryKey(), | |
| publicKey: text('public_key').notNull(), | |
| privateKey: text('private_key').notNull(), // must be encrypted at rest or via KMS | |
| createdAt: timestamp('created_at').notNull(), | |
| expiresAt: timestamp('expires_at'), | |
| isActive: boolean('is_active').notNull().default(true), | |
| }); |
🤖 Prompt for AI Agents
In apps/server/src/db/schema.ts around lines 169 to 174, the jwks table
currently stores private keys in plaintext, which poses a security risk. To fix
this, modify the schema to store the privateKey field encrypted at rest, for
example by integrating PostgreSQL's pgcrypto or a key management service.
Additionally, add expires_at and is_active columns to support key rotation.
Ensure that access controls are applied to restrict table access to only
necessary components. Finally, update the authentication logic in
apps/server/src/main.ts to load and decrypt private keys just-in-time while
continuing to use in-memory JWK sets for public key verification.
| createRateLimiterMiddleware({ | ||
| limiter: Ratelimit.slidingWindow(60, '1m'), | ||
| generatePrefix: ({ session }) => `ratelimit:get-connections-${session?.user.id}`, | ||
| generatePrefix: ({ sessionUser }) => `ratelimit:get-connections-${sessionUser?.id}`, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent null safety with sessionUser access.
The rate limiter uses optional chaining sessionUser?.id, suggesting sessionUser can be null/undefined, but this pattern isn't consistently applied throughout the file.
🤖 Prompt for AI Agents
In apps/server/src/trpc/routes/connections.ts at line 14, the use of optional
chaining with sessionUser?.id is inconsistent with other parts of the file where
sessionUser is accessed without null checks. Review all instances where
sessionUser is accessed and apply consistent null safety checks, either by using
optional chaining or by validating sessionUser before access, to ensure uniform
handling of potential null or undefined values.
| const user = ctx.sessionUser; | ||
| await db | ||
| .delete(connection) | ||
| .where(and(eq(connection.id, connectionId), eq(connection.userId, user.id))); |
There was a problem hiding this comment.
Add null safety check for sessionUser.
This mutation also needs null checking for sessionUser.
Apply this diff:
.mutation(async ({ input, ctx }) => {
const { connectionId } = input;
const { db } = ctx;
- const user = ctx.sessionUser;
+ const user = ctx.sessionUser;
+ if (!user) {
+ throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const user = ctx.sessionUser; | |
| await db | |
| .delete(connection) | |
| .where(and(eq(connection.id, connectionId), eq(connection.userId, user.id))); | |
| .mutation(async ({ input, ctx }) => { | |
| const { connectionId } = input; | |
| const { db } = ctx; | |
| const user = ctx.sessionUser; | |
| if (!user) { | |
| throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' }); | |
| } | |
| await db | |
| .delete(connection) | |
| .where( | |
| and( | |
| eq(connection.id, connectionId), | |
| eq(connection.userId, user.id), | |
| ), | |
| ); | |
| }) |
🤖 Prompt for AI Agents
In apps/server/src/trpc/routes/connections.ts around lines 71 to 74, the code
uses ctx.sessionUser without checking if it is null, which can cause runtime
errors. Add a null safety check for sessionUser before proceeding with the
delete operation. If sessionUser is null, handle it appropriately, such as
throwing an unauthorized error or returning early, to ensure the mutation does
not execute with an invalid user context.
| const elevenLabs = new ElevenLabsClient({ | ||
| apiKey: env.ELEVENLABS_API_KEY, | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify environment variable availability.
The ElevenLabs client is instantiated at module level using environment variables. Ensure these environment variables are properly validated and available at startup.
🏁 Script executed:
#!/bin/bash
# Check if ELEVENLABS_API_KEY and ELEVENLABS_AGENT_ID environment variables are properly defined
rg -A 5 -B 5 "ELEVENLABS_API_KEY|ELEVENLABS_AGENT_ID"Length of output: 4365
Ensure ELEVENLABS_API_KEY and ELEVENLABS_AGENT_ID are configured
We ran a search for both ELEVENLABS_API_KEY and ELEVENLABS_AGENT_ID and didn’t find definitions for the backend service—only the public agent ID for the client. To prevent runtime failures:
- Add
ELEVENLABS_API_KEYandELEVENLABS_AGENT_IDto your environment configuration:- Update
.env.exampleor Cloudflare Workers’ secrets. - Add entries in
docker-compose.prod.yaml(or your deployment manifest).
- Update
- Document these variables in the project README or env-template.
- Add a startup check (e.g.
if (!env.ELEVENLABS_API_KEY) throw new Error('Missing ELEVENLABS_API_KEY')) so missing keys fail fast.
🤖 Prompt for AI Agents
In apps/server/src/trpc/routes/voice/get-signed-url.ts around lines 5 to 7, the
ElevenLabsClient is instantiated using environment variables without validation.
To fix this, add checks at application startup to verify that ELEVENLABS_API_KEY
and ELEVENLABS_AGENT_ID are defined in the environment; if not, throw an error
to fail fast. Also, update your environment configuration files like
.env.example and deployment manifests such as docker-compose.prod.yaml to
include these variables, and document them in the project README or environment
template.
| // Bearer token if no session user yet | ||
| if (c.req.header('Authorization') && !session?.user) { | ||
| const token = c.req.header('Authorization')?.split(' ')[1]; | ||
|
|
||
| if (token) { | ||
| const localJwks = await auth.api.getJwks(); | ||
| const jwks = createLocalJWKSet(localJwks); | ||
|
|
||
| const { payload } = await jwtVerify(token, jwks); | ||
| const userId = payload.sub; | ||
|
|
||
| if (userId) { | ||
| c.set( | ||
| 'sessionUser', | ||
| await db.query.user.findFirst({ | ||
| where: (user, ops) => { | ||
| return ops.eq(user.id, userId); | ||
| }, | ||
| }), | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for JWT verification failures.
The bearer token authentication implementation looks good, but consider wrapping the JWT verification in a try-catch block to handle potential verification failures gracefully.
- const { payload } = await jwtVerify(token, jwks);
- const userId = payload.sub;
+ try {
+ const { payload } = await jwtVerify(token, jwks);
+ const userId = payload.sub;
- if (userId) {
- c.set(
- 'sessionUser',
- await db.query.user.findFirst({
- where: (user, ops) => {
- return ops.eq(user.id, userId);
- },
- }),
- );
- }
+ if (userId) {
+ c.set(
+ 'sessionUser',
+ await db.query.user.findFirst({
+ where: (user, ops) => {
+ return ops.eq(user.id, userId);
+ },
+ }),
+ );
+ }
+ } catch (error) {
+ console.error('Invalid JWT token:', error);
+ // Continue without setting sessionUser
+ }📝 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.
| // Bearer token if no session user yet | |
| if (c.req.header('Authorization') && !session?.user) { | |
| const token = c.req.header('Authorization')?.split(' ')[1]; | |
| if (token) { | |
| const localJwks = await auth.api.getJwks(); | |
| const jwks = createLocalJWKSet(localJwks); | |
| const { payload } = await jwtVerify(token, jwks); | |
| const userId = payload.sub; | |
| if (userId) { | |
| c.set( | |
| 'sessionUser', | |
| await db.query.user.findFirst({ | |
| where: (user, ops) => { | |
| return ops.eq(user.id, userId); | |
| }, | |
| }), | |
| ); | |
| } | |
| } | |
| } | |
| // Bearer token if no session user yet | |
| if (c.req.header('Authorization') && !session?.user) { | |
| const token = c.req.header('Authorization')?.split(' ')[1]; | |
| if (token) { | |
| const localJwks = await auth.api.getJwks(); | |
| const jwks = createLocalJWKSet(localJwks); | |
| try { | |
| const { payload } = await jwtVerify(token, jwks); | |
| const userId = payload.sub; | |
| if (userId) { | |
| c.set( | |
| 'sessionUser', | |
| await db.query.user.findFirst({ | |
| where: (user, ops) => { | |
| return ops.eq(user.id, userId); | |
| }, | |
| }), | |
| ); | |
| } | |
| } catch (error) { | |
| console.error('Invalid JWT token:', error); | |
| // Continue without setting sessionUser | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/server/src/main.ts around lines 30 to 53, the JWT verification process
lacks error handling, which can cause unhandled exceptions if verification
fails. Wrap the jwtVerify call and subsequent logic inside a try-catch block to
catch any errors during token verification. In the catch block, handle the error
gracefully, such as logging the error or ignoring the invalid token, to prevent
the application from crashing.
| console.log('[DEBUG] sending message to twilio'); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove excessive debug logging in production code.
Multiple debug console.log statements should be removed or replaced with proper logging levels to avoid cluttering production logs.
private sendToTwilio(message: TwilioSocketMessage) {
- console.log('[DEBUG] sending message to twilio');
-
if (!this.callWebSocket || this.callWebSocket.readyState !== WebSocket.OPEN) {
throw new Error('[Twilio] WebSocket not connected or not open');
}
this.callWebSocket.send(JSON.stringify(message));
- console.log('[DEBUG] sent message to twilio');
}Also applies to: 417-418
🤖 Prompt for AI Agents
In apps/server/src/services/call-service/call-service.ts at lines 410-411 and
417-418, remove the console.log debug statements or replace them with a proper
logging mechanism that supports log levels. This will prevent excessive debug
output in production and allow better control over log verbosity.
| const phoneMapping = async (phoneNumber: string) => { | ||
| console.log('[DEBUG] phoneMapping', phoneNumber); | ||
|
|
||
| const db = createDb(env.HYPERDRIVE.connectionString); | ||
|
|
||
| const obj = mapping[phoneNumber]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for phone number parameter.
The function accepts any string without validation, which could lead to security issues or unexpected behavior.
const phoneMapping = async (phoneNumber: string) => {
+ // Validate phone number format
+ const phoneRegex = /^\+[1-9]\d{1,14}$/;
+ if (!phoneRegex.test(phoneNumber)) {
+ throw new Error('Invalid phone number format');
+ }
+
console.log('[DEBUG] phoneMapping', phoneNumber);
const db = createDb(env.HYPERDRIVE.connectionString);
- const obj = mapping[phoneNumber];
+ const mapping = getPhoneMapping();
+ const obj = mapping[phoneNumber];Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/server/src/services/call-service/call-service.ts around lines 29 to 34,
the phoneMapping function accepts a phoneNumber parameter without any
validation, which can cause security risks or unexpected behavior. Add input
validation to ensure the phoneNumber parameter matches the expected format
(e.g., digits only, correct length) before proceeding with the function logic.
If the validation fails, handle it appropriately by returning an error or
rejecting the input.
| return new Promise<void>(async (resolve, reject) => { | ||
| try { | ||
| const elevenLabs = new ElevenLabsClient({ | ||
| apiKey: env.ELEVENLABS_API_KEY, | ||
| }); | ||
|
|
||
| const signedUrlResponse = await elevenLabs.conversationalAi.getSignedUrl({ | ||
| agent_id: env.ELEVENLABS_AGENT_ID, | ||
| }); | ||
|
|
||
| this.elevenLabsWebSocket = new WebSocket(signedUrlResponse.signed_url); | ||
| this.elevenLabsWebSocket.addEventListener('open', () => { | ||
| console.log(`[ElevenLabs] WebSocket connected`); | ||
|
|
||
| this.sendToElevenLabs({ | ||
| type: 'conversation_initiation_client_data', | ||
| }); | ||
|
|
||
| resolve(); | ||
| }); | ||
| this.elevenLabsWebSocket.addEventListener('message', async (event) => { | ||
| await this.handleElevenLabsMessage(event.data.toString()); | ||
| }); | ||
| this.elevenLabsWebSocket.addEventListener('error', async (event) => { | ||
| console.error(`[ElevenLabs] WebSocket error:`, event); | ||
| await this.endTwilioCall(); | ||
| }); | ||
| this.elevenLabsWebSocket.addEventListener('close', async (event) => { | ||
| console.log(`[ElevenLabs] WebSocket closed:`, event); | ||
| await this.endTwilioCall(); | ||
| }); | ||
| } catch (error) { | ||
| console.error(`[ElevenLabs] Error connecting to ElevenLabs:`, error); | ||
| reject(error); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Fix async promise executor anti-pattern.
Using an async function as a Promise executor is an anti-pattern that can lead to unhandled promise rejections and makes error handling more complex.
private async connectToElevenLabs() {
- return new Promise<void>(async (resolve, reject) => {
- try {
- const elevenLabs = new ElevenLabsClient({
- apiKey: env.ELEVENLABS_API_KEY,
- });
-
- const signedUrlResponse = await elevenLabs.conversationalAi.getSignedUrl({
- agent_id: env.ELEVENLABS_AGENT_ID,
- });
-
- this.elevenLabsWebSocket = new WebSocket(signedUrlResponse.signed_url);
- this.elevenLabsWebSocket.addEventListener('open', () => {
- console.log(`[ElevenLabs] WebSocket connected`);
-
- this.sendToElevenLabs({
- type: 'conversation_initiation_client_data',
- });
-
- resolve();
- });
- this.elevenLabsWebSocket.addEventListener('message', async (event) => {
- await this.handleElevenLabsMessage(event.data.toString());
- });
- this.elevenLabsWebSocket.addEventListener('error', async (event) => {
- console.error(`[ElevenLabs] WebSocket error:`, event);
- await this.endTwilioCall();
- });
- this.elevenLabsWebSocket.addEventListener('close', async (event) => {
- console.log(`[ElevenLabs] WebSocket closed:`, event);
- await this.endTwilioCall();
- });
- } catch (error) {
- console.error(`[ElevenLabs] Error connecting to ElevenLabs:`, error);
- reject(error);
- }
- });
+ return new Promise<void>((resolve, reject) => {
+ const elevenLabs = new ElevenLabsClient({
+ apiKey: env.ELEVENLABS_API_KEY,
+ });
+
+ elevenLabs.conversationalAi.getSignedUrl({
+ agent_id: env.ELEVENLABS_AGENT_ID,
+ }).then(signedUrlResponse => {
+ this.elevenLabsWebSocket = new WebSocket(signedUrlResponse.signed_url);
+ this.elevenLabsWebSocket.addEventListener('open', () => {
+ console.log(`[ElevenLabs] WebSocket connected`);
+
+ this.sendToElevenLabs({
+ type: 'conversation_initiation_client_data',
+ });
+
+ resolve();
+ });
+ this.elevenLabsWebSocket.addEventListener('message', async (event) => {
+ await this.handleElevenLabsMessage(event.data.toString());
+ });
+ this.elevenLabsWebSocket.addEventListener('error', async (event) => {
+ console.error(`[ElevenLabs] WebSocket error:`, event);
+ await this.endTwilioCall();
+ });
+ this.elevenLabsWebSocket.addEventListener('close', async (event) => {
+ console.log(`[ElevenLabs] WebSocket closed:`, event);
+ await this.endTwilioCall();
+ });
+ }).catch(error => {
+ console.error(`[ElevenLabs] Error connecting to ElevenLabs:`, error);
+ reject(error);
+ });
+ });
}🧰 Tools
🪛 Biome (1.9.4)
[error] 192-227: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
🤖 Prompt for AI Agents
In apps/server/src/services/call-service/call-service.ts around lines 192 to
227, the Promise executor function is declared async, which is an anti-pattern
and can cause unhandled promise rejections. To fix this, remove the async
keyword from the Promise executor and instead handle asynchronous operations
inside the executor using standard promise chaining or nested async functions.
Ensure that all asynchronous calls inside the executor are properly awaited or
handled with then/catch, and that resolve and reject are called appropriately
without relying on the executor being async.
| if (!this.elevenLabsWebSocket || this.elevenLabsWebSocket.readyState !== WebSocket.OPEN) { | ||
| // console.warn('[ElevenLabs] WebSocket not connected or not open, skipping message'); | ||
|
|
||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add proper error handling and logging for WebSocket state.
The current implementation silently returns without indication when the WebSocket is not available, which makes debugging difficult.
private sendToElevenLabs(message: ElevenLabsOutgoingSocketMessage) {
if (!this.elevenLabsWebSocket || this.elevenLabsWebSocket.readyState !== WebSocket.OPEN) {
- // console.warn('[ElevenLabs] WebSocket not connected or not open, skipping message');
-
+ console.warn('[ElevenLabs] WebSocket not connected or not open, skipping message:', message.type);
return;
}📝 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 (!this.elevenLabsWebSocket || this.elevenLabsWebSocket.readyState !== WebSocket.OPEN) { | |
| // console.warn('[ElevenLabs] WebSocket not connected or not open, skipping message'); | |
| return; | |
| } | |
| if (!this.elevenLabsWebSocket || this.elevenLabsWebSocket.readyState !== WebSocket.OPEN) { | |
| console.warn('[ElevenLabs] WebSocket not connected or not open, skipping message:', message.type); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In apps/server/src/services/call-service/call-service.ts around lines 231 to
235, the code silently returns when the WebSocket is not connected or open,
which hinders debugging. Add proper error handling by logging a warning or error
message indicating the WebSocket state issue before returning. This will provide
visibility into why the message sending is skipped.
…on across multiple routes
…ools, and enhance conversation history management
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
apps/server/src/services/call-service/call-service.ts (5)
21-26: Remove hardcoded credentials from source code.The hardcoded phone number and connection ID create security and maintainability issues, as noted in previous reviews.
29-34: Add input validation for phone number parameter.The function accepts any string without validation, which could lead to security issues or unexpected behavior, as noted in previous reviews.
180-215: Fix async promise executor anti-pattern.Using an async function as a Promise executor is an anti-pattern that can lead to unhandled promise rejections, as noted in previous reviews and confirmed by static analysis.
🧰 Tools
🪛 Biome (1.9.4)
[error] 180-215: Promise executor functions should not be
async.(lint/suspicious/noAsyncPromiseExecutor)
219-223: Add proper error handling and logging for WebSocket state.The current implementation silently returns without indication when the WebSocket is not available, as noted in previous reviews.
398-399: Remove excessive debug logging in production code.Multiple debug console.log statements should be removed or replaced with proper logging levels, as noted in previous reviews.
Also applies to: 405-405, 414-414, 444-444
🧹 Nitpick comments (3)
apps/server/src/services/call-service/call-service.ts (3)
272-273: Use optional chain for cleaner conditional logic.The multiple condition checks can be simplified using optional chaining.
Apply this diff:
- if ( - data.client_tool_call && - data.client_tool_call.tool_name && - data.client_tool_call.tool_call_id - ) { + if (data.client_tool_call?.tool_name && data.client_tool_call?.tool_call_id) {🧰 Tools
🪛 Biome (1.9.4)
[error] 272-273: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
366-374: Remove commented out code.Dead code should be removed to improve maintainability and readability.
Apply this diff:
- // get driver and connection id - // switch (toolName) { - // case 'get_thread': - // break; - // case 'list_threads': - // break; - // default: - // console.warn('[ElevenLabs] Unhandled tool call:', toolName); - // }
454-483: Remove large commented code block.This commented out method implementation should be removed if it's no longer needed, or converted to proper documentation if it provides valuable context.
Apply this diff:
- // private async getMcpToolSet(): Promise<Record<string, Tool>> { - // if (!this.mcpClient) { - // throw new Error('[Twilio] MCP client not connected'); - // } - - // // Retrieve tool metadata from MCP - // const { tools: mcpTools } = (await this.mcpClient.listTools()) as unknown as { - // tools: Array<{ - // name: string; - // description?: string; - // // The schema is JSON-schema – we treat it as unknown for now - // inputSchema?: unknown; - // }>; - // }; - - // // Convert to ai-sdk ToolSet - // return mcpTools.reduce<Record<string, Tool>>((acc, { name, description }) => { - // acc[name] = { - // description, - // // We can't infer the exact shape here – allow any parameters - // parameters: z.any(), - // execute: async (args) => { - // if (!this.mcpClient) throw new Error('[Twilio] MCP client not connected'); - // return this.mcpClient.callTool({ name, args }); - // }, - // } as Tool; - - // return acc; - // }, {}); - // }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/mail/components/ui/app-sidebar.tsx(2 hunks)apps/server/src/lib/auth.ts(2 hunks)apps/server/src/main.ts(2 hunks)apps/server/src/routes/autumn.ts(1 hunks)apps/server/src/services/call-service/call-service.ts(1 hunks)apps/server/src/trpc/routes/ai/compose.ts(1 hunks)apps/server/src/trpc/routes/notes.ts(4 hunks)apps/server/src/trpc/routes/settings.ts(4 hunks)apps/server/src/trpc/routes/shortcut.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/server/src/routes/autumn.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/server/src/main.ts
- apps/server/src/lib/auth.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/server/src/trpc/routes/notes.ts (1)
apps/server/src/db/schema.ts (1)
note(120-132)
apps/server/src/trpc/routes/shortcut.ts (1)
apps/server/src/db/schema.ts (1)
userHotkeys(60-67)
apps/server/src/trpc/routes/settings.ts (2)
apps/server/src/lib/schemas.ts (1)
defaultUserSettings(37-47)apps/server/src/db/schema.ts (1)
userSettings(134-143)
🪛 Biome (1.9.4)
apps/server/src/services/call-service/call-service.ts
[error] 180-215: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 272-273: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (9)
apps/server/src/trpc/routes/shortcut.ts (1)
14-19: LGTM! Clean session context refactor.The changes correctly update the context destructuring and user ID access to use the simplified
sessionUserproperty instead of the nestedsession.userstructure. The database operation logic remains identical and the user ID is accessed consistently.apps/server/src/trpc/routes/ai/compose.ts (1)
130-134: LGTM! Consistent session context update.The changes properly update the context destructuring and username access to use the simplified
sessionUserproperty. The functionality remains identical - the same username value is passed to thecomposeEmailfunction, maintaining the AI composition behavior.apps/server/src/trpc/routes/notes.ts (1)
12-83: LGTM! Systematic session context refactor across all note operations.All changes consistently update from
ctx.session.user.idtoctx.sessionUser.idacross the five note operations (list, create, update, delete, reorder). This maintains the same authorization logic ensuring users can only access their own notes, while simplifying the context access pattern.apps/server/src/trpc/routes/settings.ts (1)
12-68: LGTM! Well-implemented session context refactor with proper null handling.The changes systematically update all session access patterns while maintaining proper error handling:
- Rate limiter uses safe optional chaining with
sessionUser?.id- Public procedure includes appropriate null check for
sessionUser- Private procedure correctly assumes
sessionUserexists due to authorization middleware- All database operations maintain the same user-scoped filtering logic
- Default settings fallback behavior is preserved
The refactor simplifies context access while maintaining all existing functionality and proper authorization boundaries.
apps/server/src/services/call-service/call-service.ts (3)
64-78: Well-structured class initialization.The CallService class has proper property initialization and dependency injection through the constructor. The use of private properties with appropriate nullable types shows good design.
135-177: Excellent message handling with proper error handling.The Twilio and ElevenLabs message handlers demonstrate good practices:
- Use of Zod schemas for validation
- Comprehensive error handling with specific ZodError handling
- Clear logging for debugging
- Proper conversation history maintenance
Also applies to: 228-312
485-502: Good MCP client setup with proper headers.The MCP connection setup correctly includes the phone number in headers and uses proper URL construction. This enables the backend to associate the call with the correct user context.
apps/mail/components/ui/app-sidebar.tsx (2)
48-48: Import reordering is clean.The import statement reordering maintains good organization without affecting functionality.
280-280: Proper integration of CallInboxDialog component.The conditional rendering logic correctly shows the phone verification dialog when:
- User's phone number is not verified
- Session is not pending
This aligns well with the AI phone assistant feature implementation and provides a good user experience by prompting for phone verification when needed.
…Id instead of phoneNumber for improved authorization
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
apps/server/src/services/mcp-service/mcp.ts (1)
39-39: Fix the empty object type issue.Using
{}as a type is discouraged because it means "any non-nullable value". This was flagged in previous reviews and by static analysis.Apply this diff to use a more specific type:
-export class ZeroMCP extends McpAgent<typeof env, {}, { connectionId: string }> { +export class ZeroMCP extends McpAgent<typeof env, unknown, { connectionId: string }> {🧰 Tools
🪛 Biome (1.9.4)
[error] 39-39: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
apps/server/src/services/call-service/call-service.ts (3)
135-172: Fix the async promise executor anti-pattern.Using an async function as a Promise executor is an anti-pattern that can lead to unhandled promise rejections. This was flagged in previous reviews and by static analysis.
Apply this diff to fix the issue:
private async connectToElevenLabs() { - return new Promise<void>(async (resolve, reject) => { - try { - const elevenLabs = new ElevenLabsClient({ - apiKey: env.ELEVENLABS_API_KEY, - }); - - const signedUrlResponse = await elevenLabs.conversationalAi.getSignedUrl({ - agent_id: env.ELEVENLABS_AGENT_ID, - }); - - this.elevenLabsWebSocket = new WebSocket(signedUrlResponse.signed_url); - this.elevenLabsWebSocket.addEventListener('open', () => { - console.log(`[ElevenLabs] WebSocket connected`); - - this.sendToElevenLabs({ - type: 'conversation_initiation_client_data', - }); - - resolve(); - }); - this.elevenLabsWebSocket.addEventListener('message', async (event) => { - await this.handleElevenLabsMessage(event.data.toString()); - }); - this.elevenLabsWebSocket.addEventListener('error', async (event) => { - console.error(`[ElevenLabs] WebSocket error:`, event); - await this.endTwilioCall(); - }); - this.elevenLabsWebSocket.addEventListener('close', async (event) => { - console.log(`[ElevenLabs] WebSocket closed:`, event); - await this.endTwilioCall(); - }); - } catch (error) { - console.error(`[ElevenLabs] Error connecting to ElevenLabs:`, error); - reject(error); - } - }); + return new Promise<void>((resolve, reject) => { + const elevenLabs = new ElevenLabsClient({ + apiKey: env.ELEVENLABS_API_KEY, + }); + + elevenLabs.conversationalAi.getSignedUrl({ + agent_id: env.ELEVENLABS_AGENT_ID, + }).then(signedUrlResponse => { + this.elevenLabsWebSocket = new WebSocket(signedUrlResponse.signed_url); + this.elevenLabsWebSocket.addEventListener('open', () => { + console.log(`[ElevenLabs] WebSocket connected`); + + this.sendToElevenLabs({ + type: 'conversation_initiation_client_data', + }); + + resolve(); + }); + this.elevenLabsWebSocket.addEventListener('message', async (event) => { + await this.handleElevenLabsMessage(event.data.toString()); + }); + this.elevenLabsWebSocket.addEventListener('error', async (event) => { + console.error(`[ElevenLabs] WebSocket error:`, event); + await this.endTwilioCall(); + }); + this.elevenLabsWebSocket.addEventListener('close', async (event) => { + console.log(`[ElevenLabs] WebSocket closed:`, event); + await this.endTwilioCall(); + }); + }).catch(error => { + console.error(`[ElevenLabs] Error connecting to ElevenLabs:`, error); + reject(error); + }); + }); }🧰 Tools
🪛 Biome (1.9.4)
[error] 136-171: Promise executor functions should not be
async.(lint/suspicious/noAsyncPromiseExecutor)
174-182: Add warning message for better debugging.The silent return when WebSocket is not connected makes debugging difficult. This was flagged in previous reviews.
Apply this diff to improve debugging:
private sendToElevenLabs(message: ElevenLabsOutgoingSocketMessage) { if (!this.elevenLabsWebSocket || this.elevenLabsWebSocket.readyState !== WebSocket.OPEN) { - // console.warn('[ElevenLabs] WebSocket not connected or not open, skipping message'); - + console.warn('[ElevenLabs] WebSocket not connected or not open, skipping message:', message.type || 'unknown'); return; }
333-362: Remove excessive debug logging.The debug logging in
sendToTwiliowas flagged in previous reviews as excessive for production code.Apply this diff to remove excessive logging:
private sendToTwilio(message: TwilioSocketMessage) { - console.log('[DEBUG] sending message to twilio'); - if (!this.callWebSocket || this.callWebSocket.readyState !== WebSocket.OPEN) { throw new Error('[Twilio] WebSocket not connected or not open'); } this.callWebSocket.send(JSON.stringify(message)); - console.log('[DEBUG] sent message to twilio'); }
🧹 Nitpick comments (2)
apps/server/src/services/mcp-service/mcp.ts (1)
118-175: Clean up commented code and improve error messages.The implementation is solid, but contains extensive commented code that should be removed. Also, the error message could be more specific.
Apply this diff to clean up the implementation:
}; - - // const response = await env.VECTORIZE.getByIds([s.threadId]); - // if (response.length && response?.[0]?.metadata?.['content']) { - // const content = response[0].metadata['content'] as string; - // const shortResponse = await env.AI.run('@cf/facebook/bart-large-cnn', { - // input_text: content, - // }); - // return { - // content: [ - // { - // type: 'text', - // text: shortResponse.summary, - // }, - // ], - // }; - // } - // return { - // content: [ - // { - // type: 'text', - // text: `Subject: ${thread.latest?.subject}`, - // }, - // ], - // }; } catch (error) { console.error('[DEBUG] getThread error', error); return { content: [ { type: 'text', - text: 'Failed to get thread', + text: `Failed to get thread ${s.threadId}: ${error instanceof Error ? error.message : 'Unknown error'}`, }, ], };apps/server/src/services/call-service/call-service.ts (1)
270-331: Improve null safety and clean up commented code.The tool call handling has a minor safety issue and contains commented code that should be removed.
Apply this diff to improve the implementation:
if ( data.client_tool_call && - data.client_tool_call.tool_name && - data.client_tool_call.tool_call_id + data.client_tool_call?.tool_name && + data.client_tool_call?.tool_call_id ) {Also remove the commented code at lines 322-330:
- // get driver and connection id - // switch (toolName) { - // case 'get_thread': - // break; - // case 'list_threads': - // break; - // default: - // console.warn('[ElevenLabs] Unhandled tool call:', toolName); - // }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/server/src/routes/ai.ts(1 hunks)apps/server/src/services/call-service/call-service.ts(1 hunks)apps/server/src/services/mcp-service/mcp.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/server/src/routes/ai.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/server/src/services/mcp-service/mcp.ts
[error] 39-39: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
apps/server/src/services/call-service/call-service.ts
[error] 136-171: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 228-229: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (15)
apps/server/src/services/mcp-service/mcp.ts (8)
12-37: LGTM! Solid database integration and error handling.The
getDriverFromConnectionIdfunction properly retrieves connection details from the database and creates an authenticated driver with appropriate error handling for missing connections or tokens.
49-69: LGTM! AI-powered Gmail search query generation.The
buildGmailSearchQuerytool effectively uses OpenAI's GPT-4o model to generate Gmail search queries with proper system prompts and input validation.
71-116: LGTM! Comprehensive email thread listing with proper pagination.The
listThreadstool provides robust functionality with optional filtering, pagination, and proper error handling for empty results.
177-221: LGTM! Proper implementation of read/unread state management.Both
markThreadsReadandmarkThreadsUnreadtools correctly use the Gmail API's label system to manage thread read states.
223-246: LGTM! Flexible label modification functionality.The
modifyLabelstool provides a clean interface for adding and removing labels with proper validation and helpful success messages.
248-275: LGTM! Useful utility tools for context and labels.Both
getCurrentDateandgetUserLabelstools provide essential context information that would be valuable for AI-driven email management.
277-341: LGTM! Comprehensive label management with proper error handling.Both
getLabelandcreateLabeltools provide complete label management functionality with appropriate error handling for creation failures.
343-409: LGTM! Consistent bulk operations with proper error handling.Both
bulkDeleteandbulkArchivetools implement standard Gmail operations correctly using label modifications, with appropriate error handling and user feedback.apps/server/src/services/call-service/call-service.ts (7)
16-34: LGTM! Well-structured class with proper validation.The class constructor properly validates required environment variables and initializes the Twilio client with appropriate error handling.
36-52: LGTM! Comprehensive call initialization sequence.The
startCallmethod properly orchestrates the connection setup including Twilio call fetching, MCP client initialization, and ElevenLabs connection.
54-69: LGTM! Proper cleanup in call termination.Both
stopCallandendTwilioCallmethods handle cleanup appropriately by closing connections and updating call status via Twilio API.
71-133: LGTM! Robust WebSocket event handling with comprehensive error management.The Twilio message handling includes proper event routing, schema validation with Zod, and detailed error logging for debugging purposes.
184-268: LGTM! Comprehensive ElevenLabs message handling with proper conversation tracking.The message handling covers all ElevenLabs message types with appropriate logging and conversation history management for both user and assistant messages.
🧰 Tools
🪛 Biome (1.9.4)
[error] 228-229: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
364-408: LGTM! Robust AI response generation with proper error handling.The AI response generation properly integrates with the MCP client, manages conversation history, and includes comprehensive error handling with fallback responses.
441-458: LGTM! Clean MCP client connection setup.The MCP connection method properly constructs the transport with required headers for connection ID authentication.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/server/src/db/migrations/0028_worried_molecule_man.sql (1)
8-8: Clarify default application forsettingscolumn and update existing rows
TheALTER TABLE ... SET DEFAULTonly affects new records; existingmail0_user_settingsrows will retain their oldsettings. If the intent is to backfill, add anUPDATEstatement or a separate backfill migration.Example backfill:
UPDATE "mail0_user_settings" SET "settings" = '{"language":"en","timezone":"UTC","dynamicContent":false,"externalImages":true,"customPrompt":"","trustedSenders":[],"isOnboarded":false,"colorTheme":"system","zeroSignature":true}'::jsonb WHERE "settings" IS NULL;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/server/src/db/migrations/0028_worried_molecule_man.sql(1 hunks)apps/server/src/db/migrations/meta/0028_snapshot.json(1 hunks)apps/server/src/db/migrations/meta/_journal.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/server/src/db/migrations/meta/_journal.json
- apps/server/src/db/migrations/meta/0028_snapshot.json
| CREATE TABLE "mail0_jwks" ( | ||
| "id" text PRIMARY KEY NOT NULL, | ||
| "public_key" text NOT NULL, | ||
| "private_key" text NOT NULL, | ||
| "created_at" timestamp NOT NULL | ||
| ); |
There was a problem hiding this comment.
Encrypt private keys at rest
Persisting private_key as plain text poses a security risk. Consider encrypting this column using Postgres’s pgcrypto (e.g. pgp_sym_encrypt) or storing keys in a dedicated secrets manager/vault.
🤖 Prompt for AI Agents
In apps/server/src/db/migrations/0028_worried_molecule_man.sql at lines 1 to 6,
the private_key column is stored as plain text, which is a security risk. Modify
the migration to encrypt the private_key column using Postgres's pgcrypto
extension, such as applying pgp_sym_encrypt on insert and pgp_sym_decrypt on
select, or alternatively, refactor the design to store private keys securely in
a dedicated secrets manager or vault instead of the database.
🛠️ Refactor suggestion
Use native UUID type for id and default timestamp with timezone
Switching id to UUID with a default generation function and using TIMESTAMPTZ for created_at with a default of now() will improve consistency and avoid relying on external UUID generation.
Proposed diff:
-CREATE TABLE "mail0_jwks" (
- "id" text PRIMARY KEY NOT NULL,
+CREATE TABLE "mail0_jwks" (
+ "id" UUID PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL,
"public_key" text NOT NULL,
"private_key" text NOT NULL,
- "created_at" timestamp NOT NULL
+ "created_at" TIMESTAMPTZ NOT NULL DEFAULT now()
);Note: ensure the
pgcryptoextension (oruuid-ossp) is enabled before usinggen_random_uuid().
🤖 Prompt for AI Agents
In apps/server/src/db/migrations/0028_worried_molecule_man.sql lines 1 to 6,
change the "id" column type from text to native UUID and set its default value
to gen_random_uuid() to generate UUIDs within the database. Also, update the
"created_at" column type to TIMESTAMPTZ and set its default to now() to store
timestamps with timezone. Before applying these changes, ensure the pgcrypto
extension is enabled in the database to support gen_random_uuid().
| "private_key" text NOT NULL, | ||
| "created_at" timestamp NOT NULL | ||
| ); | ||
| --> statement-breakpoint |
There was a problem hiding this comment.
Fix invalid SQL comment syntax
The line --> statement-breakpoint uses -->, which Postgres interprets as an operator rather than a comment. Replace it with a proper -- comment or remove it if not required.
Proposed diff:
---> statement-breakpoint
+-- statement-breakpointCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/server/src/db/migrations/0028_worried_molecule_man.sql at line 7, the
line uses an invalid SQL comment syntax `--> statement-breakpoint`. Replace
`-->` with the correct single-line comment syntax `--` to make it a valid
Postgres comment, or remove the line entirely if the comment is unnecessary.
2 participants
…essing
READ CAREFULLY THEN REMOVE
Remove bullet points that are not relevant.
PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.
Description
Please provide a clear description of your changes.
Type of Change
Please delete options that are not relevant.
Areas Affected
Please check all that apply:
Testing Done
Describe the tests you've done:
Security Considerations
For changes involving data or authentication:
Checklist
Additional Notes
Add any other context about the pull request here.
Screenshots/Recordings
Add screenshots or recordings here if applicable.
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit
New Features
Improvements
Chores