feat: implement 1Wire AI Cold Caller (Sarah)#102
Conversation
- Replaced Express with Fastify framework for low RAM environments. - Developed `dripService` with Smart Drip MT scheduling, concurrency locking, and status callbacks. - Re-engineered `openaiRealtime` with 10s wait on `end_call`, `try/catch` tool arguments, connection checks (`isOpenAiConnected`, `isTwilioStarted`), and real-time audio append processing. - Setup `emailService` for NodeMailer to log reports and leads immediately. - Modified prompts and logic to enforce "Trifecta" + exact time collection explicitly preventing the term 'chat'. - Implemented `router.js` and `callController.js` logic for `callerId` and `mode` propagation into Twilio media streams. - Updated documentation and clean-up of obsolete services. Co-authored-by: ZyosxD <35784430+ZyosxD@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the voice bot into a Fastify-based automated cold caller named Sarah, introducing an outbound 'Smart Drip' engine and email notifications. The code review identifies several critical issues, including the use of synchronous file operations that could block the event loop during real-time audio processing and a fragile initialization sequence in the WebSocket handler. Furthermore, feedback suggests hardening the Twilio request validation to avoid security risks and improving the drip engine's error handling to ensure client statuses are correctly managed during call failures or server restarts.
| const data = fs.readFileSync(clientsPath, 'utf8'); | ||
| clients = JSON.parse(data); | ||
| } catch (err) { | ||
| logger.error('Error reading clients.json:', err); | ||
| return; | ||
| } | ||
|
|
||
| const nextClientIndex = clients.findIndex(c => c.status === 'PENDING'); | ||
|
|
||
| if (nextClientIndex === -1) { | ||
| logger.info('No pending clients to call in Smart Drip.'); | ||
| return; | ||
| } | ||
|
|
||
| const client = clients[nextClientIndex]; | ||
|
|
||
| isCallActive = true; | ||
| clients[nextClientIndex].status = 'CALLED'; | ||
|
|
||
| try { | ||
| fs.writeFileSync(clientsPath, JSON.stringify(clients, null, 2)); |
There was a problem hiding this comment.
Using synchronous file operations like fs.readFileSync and fs.writeFileSync inside the polling interval blocks the Node.js event loop. In a real-time application handling audio streams, this can cause latency spikes and degraded audio quality. Consider using fs.promises for asynchronous I/O.
| appendToFile(filePath, dataObj) { | ||
| try { | ||
| const data = fs.readFileSync(filePath, 'utf8'); | ||
| const json = JSON.parse(data); | ||
| json.push(dataObj); | ||
| fs.writeFileSync(filePath, JSON.stringify(json, null, 2)); | ||
| } catch (e) { | ||
| logger.error(`Error appending to ${filePath}`, e); | ||
| } | ||
| } |
There was a problem hiding this comment.
The appendToFile method performs synchronous read and write operations on every log entry. This blocks the event loop and becomes increasingly inefficient as the JSON files grow. For a real-time service, it is better to use asynchronous operations or an append-only format like NDJSON to avoid rewriting the entire file.
| appendToFile(filePath, dataObj) { | |
| try { | |
| const data = fs.readFileSync(filePath, 'utf8'); | |
| const json = JSON.parse(data); | |
| json.push(dataObj); | |
| fs.writeFileSync(filePath, JSON.stringify(json, null, 2)); | |
| } catch (e) { | |
| logger.error(`Error appending to ${filePath}`, e); | |
| } | |
| } | |
| async appendToFile(filePath, dataObj) { | |
| try { | |
| const data = await fs.promises.readFile(filePath, 'utf8'); | |
| const json = JSON.parse(data); | |
| json.push(dataObj); | |
| await fs.promises.writeFile(filePath, JSON.stringify(json, null, 2)); | |
| } catch (e) { | |
| logger.error(`Error appending to ${filePath}`, e); | |
| } | |
| } |
|
|
||
| // Provide the start event to initialize streams immediately | ||
| // after connect resolves, though wait till we're sure it's constructed | ||
| setTimeout(() => openAIService.handleTwilioMedia(data), 50); |
There was a problem hiding this comment.
The 50ms setTimeout is a fragile way to handle initialization and introduces a race condition. It is better to have openAIService.connect() return a Promise that resolves when the connection is established, and then ensure it is resolved before calling handleTwilioMedia.
| // Skip validation in local development if public URL is not set or localhost | ||
| if (!config.server.publicUrl || config.server.publicUrl.includes('localhost')) { | ||
| return next(); | ||
| if (!config.server.publicUrl || config.server.publicUrl.includes('localhost') || config.server.publicUrl.includes('ngrok')) { |
There was a problem hiding this comment.
Bypassing Twilio signature validation for any URL containing 'ngrok' is a security risk if accidentally left in a production-like environment. It is safer to use an environment variable check (e.g., process.env.NODE_ENV !== 'production') to disable validation during local development.
| } catch (err) { | ||
| logger.error('Error initiating Twilio call:', err); | ||
| isCallActive = false; | ||
| activeCallSid = null; | ||
| } |
There was a problem hiding this comment.
If the Twilio call initiation fails, the client's status remains 'CALLED' in clients.json (set at line 68). This prevents the drip engine from retrying the call. The status should be reset to 'PENDING' or marked as 'FAILED' in the catch block to allow for proper error handling or retries.
| } catch (err) { | |
| logger.error('Error initiating Twilio call:', err); | |
| isCallActive = false; | |
| activeCallSid = null; | |
| } | |
| } catch (err) { | |
| logger.error('Error initiating Twilio call:', err); | |
| // Reset status so it can be retried | |
| clients[nextClientIndex].status = 'PENDING'; | |
| fs.writeFileSync(clientsPath, JSON.stringify(clients, null, 2)); | |
| isCallActive = false; | |
| activeCallSid = null; | |
| } |
| let isCallActive = false; | ||
| let activeCallSid = null; |
There was a problem hiding this comment.
The isCallActive and activeCallSid state is stored in memory and is not persistent. If the server restarts, the lock is lost, but the client's status in clients.json remains 'CALLED'. This can lead to an inconsistent state where calls are not resumed or retried correctly after a crash.
1 participant
This pull request delivers the massive core overhaul required to transform the Voice Bot into "Sarah", the 1Wire Cold Calling AI Assistant.
Key additions:
v5) natively equipped with@fastify/websocket(v11) for streamlined Realtime WS bridging and footprint minimization.dripService): Integrated an outbound cron-like polling interval (setInterval) that securely handles database interaction and fires calls strictly between designated MT windows while applying strict locking controls to prevent parallel collisions.try/catchtool handling. A 10-second delay was added toend_callto cleanly vocalize goodbyes prior to socket closure.emailService): Centralized local logging intosrc/data/*.jsonmapped to immediate NodeMailer broadcasts configured for green/orange emoji flagging on success or reports.PR created automatically by Jules for task 17982069199215342893 started by @ZyosxD