feat(voice): wave_voice_converse — the agent-facing voice-agent tool - #96
Conversation
…voice agent Drive a full headless conversation (bind -> audio-in WS -> TTS WS -> PCM out) with no browser/WebRTC. Same transport the CLI (harness/voice-cli.mjs) uses, so an agent can exercise the voice loop from anywhere. Auth is the edge's internal seal (WAVE_INTERNAL_SECRET) + WAVE_REALTIME_EDGE.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 37 minutes Limit details: You’ve used the included review currently available. Your 100 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Comment |
|
Running ultrareview automatically — This PR implements a new voice conversation tool involving low-level binary packet encoding and WebSocket streaming with internal infrastructure secrets, posing risks to real-time session stability and security.. I'll post findings when complete. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_70b1d28a-8592-47a8-b31e-5ac1540088ec) |
PR Summary by QodoAdd wave_voice_converse tool for headless voice-agent conversations
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces a new voice-agent tool with substantial new runtime behavior (WebSocket connections, file I/O, audio streaming). Multiple High-severity unresolved findings exist including an arbitrary file write vulnerability, missing WebSocket polyfill for Node 18 compatibility, and unclosed WebSocket resource leaks. These warrant human review before merge. Not approved because:
Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more. |
|
I can't run this ultrareview because your workspace has reached its monthly review limit. cubic has reviewed 100,145 of the 100,000 allowed lines of code this month. Reviews resume on 4 September 2026 (in 15 days). Enable flex capacity to cover overages automatically and resume reviews now. Learn how flex capacity works. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_37d091ff-dd88-403e-9a63-c9011bf56ae7) |
|
Note Automatic reviews are paused because your team has used its included automatic processing for this billing period (headroom scales with your seat count). You can still comment "Gitar review" to run one anytime, and automatic reviews resume on their own by September 1. Add seats for more headroom. Code Review 🚫 Blocked 0 resolved / 5 findingsAdds the 🚨 Bug: Global WebSocket unavailable on Node <22, breaking the declared engine rangevoice.ts:53 uses the global Use the ws package explicitly instead of relying on a Node-version-dependent global.
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Gitar
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_3872bf36-ce73-4594-bd41-09e9f50f506c) |
Code Review by Qodo
1. WebSockets never closed
|
| export const voiceTools: WaveToolDef[] = [ | ||
| { | ||
| name: "wave_voice_converse", | ||
| description: | ||
| "Drive a full headless conversation with the WAVE voice agent: bind the agent to a room, send a WAV " + | ||
| "of the caller's speech (16-bit LE 48 kHz PCM, mono or stereo), and receive the agent's spoken reply " + | ||
| "as raw 16-bit LE 48 kHz stereo PCM written to outPath. No browser, no WebRTC.", |
There was a problem hiding this comment.
1. Missing changelog for wave_voice_converse 📘 Rule violation ⚙ Maintainability
This PR adds the user-facing wave_voice_converse tool, but CHANGELOG.md has no corresponding entry under ## [Unreleased]. This can cause user-visible changes to ship without release documentation.
Agent Prompt
## Issue description
A new user-facing tool (`wave_voice_converse`) is added, but the `CHANGELOG.md` `## [Unreleased]` section has no entry documenting it.
## Issue Context
Compliance requires documenting user-facing behavior changes (new tools/APIs/CLI options) in the Unreleased section.
## Fix Focus Areas
- CHANGELOG.md[7-10]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| function connect(url: string): Promise<WebSocket> { | ||
| return new Promise((resolve, reject) => { | ||
| const ws = new WebSocket(url); | ||
| ws.binaryType = "arraybuffer"; |
There was a problem hiding this comment.
2. Missing websocket implementation 🐞 Bug ☼ Reliability
voice.ts uses new WebSocket(url) without importing/providing a WebSocket implementation; on supported Node >=18 installs this can throw at runtime (ReferenceError) and the tool will never connect. This is especially risky because the package explicitly supports Node 18 where global WebSocket is not guaranteed.
Agent Prompt
## Issue description
`connect()` constructs `new WebSocket(url)` but no WebSocket implementation is imported or polyfilled. Because this package supports Node >=18, the tool can crash at runtime on environments where `globalThis.WebSocket` is absent.
## Issue Context
Other server entrypoints don’t install a WebSocket polyfill, and dependencies do not include a WebSocket client library.
## Fix Focus Areas
- src/tools/voice.ts[51-57]
- package.json[56-67]
## Expected fix
- Add an explicit WebSocket client implementation:
- Prefer `import { WebSocket } from "undici";` (and use that symbol), OR add `ws` as a dependency and `import WebSocket from "ws";`.
- Optionally feature-detect and throw a clear error if WebSocket is unavailable.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const total = Buffer.concat(outChunks); | ||
| if (total.length === 0) throw new Error("no TTS received (agent did not reply)"); | ||
| writeFileSync(outPath, total); | ||
| return `TTS received: ${total.length} bytes (${Math.round(total.length / bytesPerMs)} ms) → ${outPath}`; |
There was a problem hiding this comment.
3. Arbitrary file write via outpath 🐞 Bug ⛨ Security
wave_voice_converse writes raw PCM to a user-controlled outPath with `writeFileSync(outPath, total)`, enabling arbitrary file overwrite on the MCP server filesystem. Because this is an agent-facing tool, a compromised/malicious caller could clobber configs/keys or plant data in sensitive locations.
Agent Prompt
## Issue description
The tool accepts `outPath` from the caller and writes to it directly. This is an arbitrary file write primitive exposed via an agent tool.
## Issue Context
The handler passes `audioPath`/`outPath` through without validation, and the tool runs on the MCP server host.
## Fix Focus Areas
- src/tools/voice.ts[63-64]
- src/tools/voice.ts[76-77]
- src/tools/voice.ts[103-106]
- src/tools/voice.ts[116-120]
## Expected fix
Choose one:
- (Preferred) Remove filesystem paths from the tool interface:
- Accept the WAV bytes as base64 (or MCP attachment if available), and return the PCM bytes as base64 in the tool response.
- If paths must remain:
- Enforce a configured safe directory (e.g., `VOICE_IO_DIR`) and reject any path that escapes it (`..`, absolute paths, symlinks).
- Use exclusive create / safe overwrite semantics if overwriting is not intended.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const ttl = Date.now() + 60000; | ||
| while (!closed && Date.now() < ttl) await sleep(250); | ||
|
|
||
| const total = Buffer.concat(outChunks); | ||
| if (total.length === 0) throw new Error("no TTS received (agent did not reply)"); | ||
| writeFileSync(outPath, total); | ||
| return `TTS received: ${total.length} bytes (${Math.round(total.length / bytesPerMs)} ms) → ${outPath}`; |
There was a problem hiding this comment.
4. Websockets never closed 🐞 Bug ☼ Reliability
The tool opens audioIn and tts WebSockets but never closes either socket in success, error, or timeout paths. This can leak open connections and keep resources alive across tool invocations (especially when the TTS socket never closes within the TTL).
Agent Prompt
## Issue description
`converse()` creates two WebSockets and waits up to 60s for `tts` to close, but there is no `close()`/cleanup in any path. On timeout or error, both sockets can remain open.
## Issue Context
This tool runs in a long-lived MCP server process; leaked sockets can accumulate and degrade reliability.
## Fix Focus Areas
- src/tools/voice.ts[77-83]
- src/tools/voice.ts[100-107]
## Expected fix
- Wrap the core logic in `try { ... } finally { ... }` and call:
- `audioIn.close()` when done sending (or after receiving reply)
- `tts.close()` on timeout/error
- Also handle `audioIn.onclose`/`onerror` to fail fast, and on TTL expiry explicitly throw a timeout error after closing both sockets.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const CHUNK = 32000; | ||
| const bytesPerMs = 48000 * 2 * 2 / 1000; | ||
| let seq = 0, ts = 0; | ||
| const started = Date.now(); | ||
| for (let off = 0; off < pcm.length; off += CHUNK) { | ||
| const chunk = pcm.subarray(off, off + CHUNK); | ||
| audioIn.send(encodePacket(chunk, seq++, ts)); | ||
| ts += Math.floor(chunk.length / 4); | ||
| const target = (off + chunk.length) / bytesPerMs; |
There was a problem hiding this comment.
5. Mono input timing mismatch 🐞 Bug ≡ Correctness
The tool advertises mono or stereo WAV input, but pacing and timestamping assume 48kHz stereo 16-bit frames (bytesPerMs uses 4 bytes/frame; ts += chunk.length/4). If a mono WAV is provided, timestamps and send pacing will be wrong and can break or distort the headless audio-in stream.
Agent Prompt
## Issue description
The streaming loop assumes 4 bytes per frame (stereo 16-bit) but the tool description says mono WAVs are supported. For mono input, `bytesPerMs` and `ts` are computed incorrectly.
## Issue Context
`decodeWav()` extracts only the `data` chunk and ignores the WAV `fmt ` chunk (channels/sampleRate/bitsPerSample), so the sender cannot adjust correctly.
## Fix Focus Areas
- src/tools/voice.ts[41-50]
- src/tools/voice.ts[84-92]
- src/tools/voice.ts[113-115]
## Expected fix
- Parse the WAV `fmt ` chunk and validate:
- sampleRate === 48000
- bitsPerSample === 16
- channels is either 1 or 2
- Then either:
- Reject mono input (update description accordingly), OR
- Upmix mono->stereo before sending and keep the stereo timing math, OR
- Adjust `bytesPerMs` and `ts` based on the parsed channel count.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…andom in a security context)
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_23adbecd-7409-4829-88dc-68d5a174cda9) |
Qodo Fixer🍒 Ready to be cherry-picked — ✅ Merged (0) · ☑ Fixed (5) 🔗 Fix PR: #97 This fix PR was closed automatically. Its branch is preserved so you can cherry pick the changes into the original PR. Prompt for coding agent Process — 5 fixed
|
Drive a full headless conversation (bind -> audio-in WS -> TTS WS -> PCM out) with no browser/WebRTC. Same transport as the voice CLI. Auth via WAVE_INTERNAL_SECRET + WAVE_REALTIME_EDGE (the edge's internal seal).
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
High Risk
Uses the edge internal seal (
WAVE_INTERNAL_SECRET) rather than customer auth, and reads/writes arbitrary filesystem paths supplied by the tool caller.Overview
Adds
wave_voice_converseso an agent can drive a full headless voice-agent conversation without a browser or WebRTC: bind to a room, stream a WAV of caller speech, and write the agent’s TTS reply as raw PCM.The tool talks to
WAVE_REALTIME_EDGEwithWAVE_INTERNAL_SECRET(not the customer API key), matching the existing CLI/harness transport. Input is a 48 kHz 16-bit WAV; output is raw stereo PCM atoutPath. Registered in the sharedallToolslist so stdio and SDK transports stay in parity.Reviewed by Cursor Bugbot for commit 50b28cc. Bugbot is set up for automated code reviews on this repo. Configure here.