Skip to content

feat(voice): wave_voice_converse — the agent-facing voice-agent tool - #96

Merged
yakimoto merged 4 commits into
mainfrom
feat/voice-tool
Sep 4, 2026
Merged

feat(voice): wave_voice_converse — the agent-facing voice-agent tool#96
yakimoto merged 4 commits into
mainfrom
feat/voice-tool

Conversation

@yakimoto

@yakimoto yakimoto commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

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).


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with 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_converse so 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_EDGE with WAVE_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 at outPath. Registered in the shared allTools list 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.

…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.
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your current included review allowance is based on your included PR review attempts over the past 7 days.

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 @coderabbitai review or push new commits to the PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 90da2e1a-35c2-459d-bb72-d8e271ebee87

📥 Commits

Reviewing files that changed from the base of the PR and between 19897c2 and 50b28cc.

📒 Files selected for processing (2)
  • src/tools/index.ts
  • src/tools/voice.ts

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 21, 2026

Copy link
Copy Markdown

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.

@cursor

cursor Bot commented Aug 21, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

Comment thread src/tools/voice.ts Fixed
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add wave_voice_converse tool for headless voice-agent conversations

✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Register a new agent-facing tool to run headless voice conversations (no WebRTC/browser).
• Bind to the realtime edge, stream WAV PCM via audio-in WebSocket, and collect TTS PCM from TTS
 WebSocket.
• Authenticate using internal edge seal env vars (WAVE_INTERNAL_SECRET/WAVE_REALTIME_EDGE).
Diagram

graph TD
  A(["Agent tool: wave_voice_converse"]) --> B["Realtime Edge: /agents/bind"] --> C(("Audio-in WS")) --> D["Voice Agent"]
  D --> E(("TTS WS")) --> F["Write PCM out"]
  subgraph Legend
    direction LR
    _tool(["Tool"]) ~~~ _svc["Service/API"] ~~~ _ws(("WebSocket"))
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Share transport/framing code with the existing voice CLI/harness
  • ➕ Avoids protocol drift between CLI and tool (packet framing, timestamps, chunking).
  • ➕ Centralizes fixes for edge protocol changes (tags/fields/encoding).
  • ➕ Enables reuse of WAV decode + pacing logic across callers.
  • ➖ Requires refactoring into a shared module/package and coordinating dependency boundaries.
  • ➖ May be harder to keep the MCP server bundle minimal if shared code brings extra deps.
2. Use a real protobuf implementation for packet framing
  • ➕ Eliminates hand-rolled varint/tag parsing, reducing subtle framing bugs.
  • ➕ Makes protocol evolution clearer (schema-driven).
  • ➕ Improves maintainability for future fields/versions.
  • ➖ Adds dependency/runtime overhead and potentially a build step (codegen) depending on approach.
  • ➖ May be overkill if the framing is intentionally minimal and stable.

Recommendation: The PR’s approach is reasonable for quickly enabling headless agent-driven voice loops, but the custom varint/proto-like framing is the riskiest part. If this tool is expected to be long-lived, prefer either (a) extracting shared transport/framing code used by the voice CLI/harness, or (b) adopting a protobuf schema/library for encode/decode to prevent silent drift and parsing edge cases.

Files changed (2) +132 / -0

Enhancement (2) +132 / -0
index.tsRegister voice tools in the global tool list +2/-0

Register voice tools in the global tool list

• Imports the new voice tool module and appends its tool definitions into the exported allTools list, making wave_voice_converse discoverable/usable by the tool runtime.

src/tools/index.ts

voice.tsAdd wave_voice_converse headless voice conversation tool +130/-0

Add wave_voice_converse headless voice conversation tool

• Introduces a new tool that binds a headless voice-agent session on the realtime edge, streams input WAV PCM over an audio-in WebSocket with custom packet framing, and collects TTS audio from a separate WebSocket. Writes the concatenated TTS audio as raw 16-bit LE 48kHz stereo PCM to outPath and authenticates via internal seal env vars.

src/tools/voice.ts

@macroscopeapp

macroscopeapp Bot commented Aug 21, 2026

Copy link
Copy Markdown

Approvability

Verdict: 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:

  • Credit balance exhausted. Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more.

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 21, 2026

Copy link
Copy Markdown

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:

Learn more →

@cursor

cursor Bot commented Aug 21, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@gitar-bot

gitar-bot Bot commented Aug 21, 2026

Copy link
Copy Markdown

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.
Learn more

Code Review 🚫 Blocked 0 resolved / 5 findings

Adds the wave_voice_converse agent tool for headless voice conversations, but contains critical compatibility issues, unclosed WebSockets, and arbitrary filesystem access vulnerabilities.

🚨 Bug: Global WebSocket unavailable on Node <22, breaking the declared engine range

📄 src/tools/voice.ts:51-58

voice.ts:53 uses the global WebSocket constructor with no import/polyfill, but package.json declares "engines": { "node": ">=18.0.0" }. Global WebSocket was only added to Node as a stable global in Node 22 (experimental/flagged in 20/21, absent in 18), so on any supported Node 18/20 runtime this tool will throw ReferenceError: WebSocket is not defined for every invocation. Either import the ws package (already likely used elsewhere in the wave-realtime-edge harness) or raise the engines.node requirement to >=22 and document it.

Use the ws package explicitly instead of relying on a Node-version-dependent global.
import WebSocket from "ws";
// ...
function connect(url: string): Promise<WebSocket> {
  return new Promise((resolve, reject) => {
    const ws = new WebSocket(url);
    ws.binaryType = "arraybuffer" as any;
    ws.once("open", () => resolve(ws));
    ws.once("error", (e) => reject(e instanceof Error ? e : new Error("WS connect failed")));
  });
}
⚠️ Bug: WebSocket connections are never closed, leaking sockets on every call and on error paths

📄 src/tools/voice.ts:76-90

In converse() (voice.ts:77-106), audioIn and tts are opened via connect() but never explicitly .close()d — the code relies on the server closing tts (setting closed = true) and simply abandons audioIn. If decodeWav/readFileSync/bind throws after the sockets are opened, or the 60s TTL expires without the server closing tts, both sockets leak indefinitely since there is no try/finally. In a long-running MCP server process handling many conversations this accumulates open sockets/handles over time.

Wrap socket usage in try/finally so both sockets are always closed, regardless of success, timeout, or thrown error.
let audioIn: WebSocket | undefined, tts: WebSocket | undefined;
try {
  [audioIn, tts] = await Promise.all([connect(json.audioInEndpoint), connect(json.ttsEndpoint)]);
  // ... existing logic using audioIn/tts ...
} finally {
  audioIn?.close();
  tts?.close();
}
⚠️ Security: audioPath/outPath allow arbitrary filesystem read/write with no path restriction

📄 src/tools/voice.ts:63 📄 src/tools/voice.ts:76 📄 src/tools/voice.ts:105

converse() (voice.ts:76, 105) calls readFileSync(audioPath) and writeFileSync(outPath, total) directly on agent-supplied strings with no validation against traversal (../), absolute paths outside an allowed sandbox, or symlinks. Since this tool is invoked by an LLM agent (potentially acting on untrusted instructions embedded in content it processes), this permits reading arbitrary files as "audio" (causing a parse error, but still a read) and writing the TTS output to any writable path on the host, e.g. overwriting config files. Constrain both paths to a dedicated working directory (e.g. resolve against a fixed base dir and reject paths that escape it via path.resolve/path.relative checks).

Resolve and validate both paths against a configured sandbox directory before any file I/O.
import { resolve, relative } from "node:path";
const SANDBOX = process.env.WAVE_VOICE_WORKDIR ?? process.cwd();
function assertInSandbox(p: string): string {
  const abs = resolve(SANDBOX, p);
  if (relative(SANDBOX, abs).startsWith("..")) throw new Error(`path escapes sandbox: ${p}`);
  return abs;
}
// then: readFileSync(assertInSandbox(audioPath)) / writeFileSync(assertInSandbox(outPath), total)
💡 Edge Case: No onerror handler after connect(); mid-stream socket errors hang until the 60s TTL

📄 src/tools/voice.ts:79-93

connect() (voice.ts:51-58) wires onerror only to reject the initial connection promise; once resolved, audioIn/tts have no onerror handler for the remainder of converse(). If either socket errors mid-conversation (e.g., edge drops the connection), the code has no signal and simply waits out the full 60s TTL loop before reporting a generic "no TTS received" error, making failures slow and hard to diagnose. Attach an error listener that flips closed/rejects early to fail fast with a clearer message.

Track socket errors during streaming and surface them instead of silently timing out.
let sockErr: Error | undefined;
tts.onerror = () => { sockErr = new Error("TTS socket error"); closed = true; };
audioIn.onerror = () => { sockErr = new Error("audio-in socket error"); };
// ... after the wait loop:
if (sockErr) throw sockErr;
💡 Edge Case: decodeWav does not validate RIFF/WAVE magic before parsing chunks

📄 src/tools/voice.ts:41-50

decodeWav() (voice.ts:41-50) skips straight to offset 12 and starts reading chunk headers without checking that the buffer begins with "RIFF"/"WAVE" magic bytes. A non-WAV or truncated file can cause readUInt32LE to read garbage lengths, potentially throwing a confusing RangeError (out of bounds) or, in pathological cases, produce a data subarray with an out-of-range end that Node silently clamps, returning corrupted/truncated PCM without any explicit error about the file being invalid.

Validate the RIFF/WAVE header up front and give a clear error for malformed input.
function decodeWav(buf: Buffer): { pcm: Buffer } {
  if (buf.length < 12 || buf.toString("ascii", 0, 4) !== "RIFF" || buf.toString("ascii", 8, 12) !== "WAVE") {
    throw new Error("not a valid WAV file (missing RIFF/WAVE header)");
  }
  // ... existing loop ...
}
🤖 Prompt for agents
Code Review: Adds the `wave_voice_converse` agent tool for headless voice conversations, but contains critical compatibility issues, unclosed WebSockets, and arbitrary filesystem access vulnerabilities.

1. 🚨 Bug: Global WebSocket unavailable on Node <22, breaking the declared engine range
   Files: src/tools/voice.ts:51-58

   voice.ts:53 uses the global `WebSocket` constructor with no import/polyfill, but package.json declares `"engines": { "node": ">=18.0.0" }`. Global `WebSocket` was only added to Node as a stable global in Node 22 (experimental/flagged in 20/21, absent in 18), so on any supported Node 18/20 runtime this tool will throw `ReferenceError: WebSocket is not defined` for every invocation. Either import the `ws` package (already likely used elsewhere in the wave-realtime-edge harness) or raise the engines.node requirement to >=22 and document it.

   Fix (Use the ws package explicitly instead of relying on a Node-version-dependent global.):
   import WebSocket from "ws";
   // ...
   function connect(url: string): Promise<WebSocket> {
     return new Promise((resolve, reject) => {
       const ws = new WebSocket(url);
       ws.binaryType = "arraybuffer" as any;
       ws.once("open", () => resolve(ws));
       ws.once("error", (e) => reject(e instanceof Error ? e : new Error("WS connect failed")));
     });
   }

2. ⚠️ Bug: WebSocket connections are never closed, leaking sockets on every call and on error paths
   Files: src/tools/voice.ts:76-90

   In `converse()` (voice.ts:77-106), `audioIn` and `tts` are opened via `connect()` but never explicitly `.close()`d — the code relies on the server closing `tts` (setting `closed = true`) and simply abandons `audioIn`. If `decodeWav`/`readFileSync`/`bind` throws after the sockets are opened, or the 60s TTL expires without the server closing `tts`, both sockets leak indefinitely since there is no `try/finally`. In a long-running MCP server process handling many conversations this accumulates open sockets/handles over time.

   Fix (Wrap socket usage in try/finally so both sockets are always closed, regardless of success, timeout, or thrown error.):
   let audioIn: WebSocket | undefined, tts: WebSocket | undefined;
   try {
     [audioIn, tts] = await Promise.all([connect(json.audioInEndpoint), connect(json.ttsEndpoint)]);
     // ... existing logic using audioIn/tts ...
   } finally {
     audioIn?.close();
     tts?.close();
   }

3. ⚠️ Security: audioPath/outPath allow arbitrary filesystem read/write with no path restriction
   Files: src/tools/voice.ts:63, src/tools/voice.ts:76, src/tools/voice.ts:105

   `converse()` (voice.ts:76, 105) calls `readFileSync(audioPath)` and `writeFileSync(outPath, total)` directly on agent-supplied strings with no validation against traversal (`../`), absolute paths outside an allowed sandbox, or symlinks. Since this tool is invoked by an LLM agent (potentially acting on untrusted instructions embedded in content it processes), this permits reading arbitrary files as "audio" (causing a parse error, but still a read) and writing the TTS output to any writable path on the host, e.g. overwriting config files. Constrain both paths to a dedicated working directory (e.g. resolve against a fixed base dir and reject paths that escape it via `path.resolve`/`path.relative` checks).

   Fix (Resolve and validate both paths against a configured sandbox directory before any file I/O.):
   import { resolve, relative } from "node:path";
   const SANDBOX = process.env.WAVE_VOICE_WORKDIR ?? process.cwd();
   function assertInSandbox(p: string): string {
     const abs = resolve(SANDBOX, p);
     if (relative(SANDBOX, abs).startsWith("..")) throw new Error(`path escapes sandbox: ${p}`);
     return abs;
   }
   // then: readFileSync(assertInSandbox(audioPath)) / writeFileSync(assertInSandbox(outPath), total)

4. 💡 Edge Case: No onerror handler after connect(); mid-stream socket errors hang until the 60s TTL
   Files: src/tools/voice.ts:79-93

   `connect()` (voice.ts:51-58) wires `onerror` only to reject the initial connection promise; once resolved, `audioIn`/`tts` have no `onerror` handler for the remainder of `converse()`. If either socket errors mid-conversation (e.g., edge drops the connection), the code has no signal and simply waits out the full 60s TTL loop before reporting a generic "no TTS received" error, making failures slow and hard to diagnose. Attach an error listener that flips `closed`/rejects early to fail fast with a clearer message.

   Fix (Track socket errors during streaming and surface them instead of silently timing out.):
   let sockErr: Error | undefined;
   tts.onerror = () => { sockErr = new Error("TTS socket error"); closed = true; };
   audioIn.onerror = () => { sockErr = new Error("audio-in socket error"); };
   // ... after the wait loop:
   if (sockErr) throw sockErr;

5. 💡 Edge Case: decodeWav does not validate RIFF/WAVE magic before parsing chunks
   Files: src/tools/voice.ts:41-50

   `decodeWav()` (voice.ts:41-50) skips straight to offset 12 and starts reading chunk headers without checking that the buffer begins with `"RIFF"`/`"WAVE"` magic bytes. A non-WAV or truncated file can cause `readUInt32LE` to read garbage lengths, potentially throwing a confusing `RangeError` (out of bounds) or, in pathological cases, produce a `data` subarray with an out-of-range end that Node silently clamps, returning corrupted/truncated PCM without any explicit error about the file being invalid.

   Fix (Validate the RIFF/WAVE header up front and give a clear error for malformed input.):
   function decodeWav(buf: Buffer): { pcm: Buffer } {
     if (buf.length < 12 || buf.toString("ascii", 0, 4) !== "RIFF" || buf.toString("ascii", 8, 12) !== "WAVE") {
       throw new Error("not a valid WAV file (missing RIFF/WAVE header)");
     }
     // ... existing loop ...
   }

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@cursor

cursor Bot commented Aug 21, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@qodo-code-review

qodo-code-review Bot commented Aug 21, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Action required

1. WebSockets never closed 🐞 Bug ☼ Reliability
Description
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).
Code

src/tools/voice.ts[R100-106]

+  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}`;
Relevance

●●● Strong

Explicit socket cleanup is a clear resource-leak fix for success, timeout, and error paths.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The code establishes both sockets, waits for tts closure, then returns/writes output without any
call to close(); on TTL expiry the loop ends but no cleanup occurs before continuing/throwing.

src/tools/voice.ts[77-83]
src/tools/voice.ts[100-107]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## 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


2. Missing WebSocket implementation 🐞 Bug ☼ Reliability
Description
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.
Code

src/tools/voice.ts[R51-54]

+function connect(url: string): Promise<WebSocket> {
+  return new Promise((resolve, reject) => {
+    const ws = new WebSocket(url);
+    ws.binaryType = "arraybuffer";
Relevance

●●● Strong

Direct Node runtime compatibility risk; missing WebSocket implementation is a deterministic
reliability fix.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new tool calls new WebSocket(url) with no import, and the package declares support for Node
>=18, so this can fail at runtime on Node 18 installations/environments without a global WebSocket.

src/tools/voice.ts[51-57]
package.json[56-67]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## 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


3. Arbitrary file write via outPath 🐞 Bug ⛨ Security
Description
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.
Code

src/tools/voice.ts[R103-106]

+  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}`;
Relevance

●● Moderate

Security concern is plausible, but repository evidence does not establish its policy for
caller-controlled output paths.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The tool schema accepts outPath and the implementation writes to it with no validation/sandboxing,
directly exposing server filesystem writes to the caller.

src/tools/voice.ts[103-106]
src/tools/voice.ts[116-120]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## 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



Remediation recommended

4. Missing CHANGELOG for wave_voice_converse 📘 Rule violation ⚙ Maintainability
Description
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.
Code

src/tools/voice.ts[R109-115]

+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.",
Relevance

●●● Strong

Recent accepted precedent requires documenting user-visible changes in CHANGELOG Unreleased.

PR-#76

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist requires user-facing changes to be reflected under CHANGELOG.md's ## [Unreleased].
The PR introduces a new tool named wave_voice_converse, while the Unreleased section currently has
no bullet entries before the next release header.

Rule 2497950: Document user-facing changes in CHANGELOG Unreleased section
src/tools/voice.ts[109-116]
CHANGELOG.md[7-10]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## 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


5. Mono input timing mismatch 🐞 Bug ≡ Correctness
Description
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.
Code

src/tools/voice.ts[R84-92]

+  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;
Relevance

●●● Strong

Hard-coded stereo timing contradicts advertised mono support and deterministically corrupts pacing
and timestamps.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The description explicitly claims mono input support, but the implementation hard-codes stereo
timing math (48000*2*2 and division by 4 bytes/frame), which is only correct for stereo 16-bit
PCM.

src/tools/voice.ts[84-92]
src/tools/voice.ts[113-115]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## 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


Grey Divider

Context sources
✅ Compliance rules (platform): 2 rules
Review mode: ⚖️ Balanced: This adds security-sensitive internal authentication and a new networked voice transport with parsing, streaming, filesystem I/O, and timeout behavior; a careful single-pass review is warranted.

Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/tools/voice.ts
Comment on lines +109 to +115
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.",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

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

Comment thread src/tools/voice.ts
Comment on lines +51 to +54
function connect(url: string): Promise<WebSocket> {
return new Promise((resolve, reject) => {
const ws = new WebSocket(url);
ws.binaryType = "arraybuffer";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

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

Comment thread src/tools/voice.ts
Comment on lines +103 to +106
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}`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

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

Comment thread src/tools/voice.ts
Comment on lines +100 to +106
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}`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

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

Comment thread src/tools/voice.ts
Comment on lines +84 to +92
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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

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

@yakimoto
yakimoto enabled auto-merge August 21, 2026 16:40
@cursor

cursor Bot commented Aug 21, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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-code-review

Copy link
Copy Markdown

Qodo Fixer

🍒 Ready to be cherry-picked — ✅ Merged (0) · ☑ Fixed (5)

Grey Divider

🔗 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

This is an automated fix prepared on a separate branch (#97). It is NOT applied to this PR.
To use it: review Fix PR #97 (https://github.com/wave-av/mcp-server/pull/97), evaluate each change critically against your local context, and cherry-pick the changes that are correct into this branch. Do not accept them blindly.
Process — 5 fixed
  • ☑ Fixed: WebSockets never closed
  • ☑ Fixed: Missing WebSocket implementation
  • ☑ Fixed: Arbitrary file write via outPath
  • ☑ Fixed: Missing CHANGELOG for wave_voice_converse
  • ☑ Fixed: Mono input timing mismatch

@yakimoto
yakimoto merged commit cfab88d into main Sep 4, 2026
18 checks passed
@yakimoto
yakimoto deleted the feat/voice-tool branch September 4, 2026 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants