diff --git a/AGENTS.md b/AGENTS.md index 0a47aff..fa4bbf5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -113,7 +113,11 @@ Translation is a fixed rulebook (`src/matrix/twilio-voice.json`), not a guess. TwiML omits one. `ConversationRelay` and `VirtualAgent` are unsupported (separate IoV). - `Stream` — Twilio's WS message schema is emulated by the translator's stream - bridge; live Bandwidth-side binding requires fixture capture. + bridge; live Bandwidth-side binding requires fixture capture. `` + children map to nested `` elements in order (Bandwidth allows + at most 12; extras are dropped with a warning). Bandwidth echoes them in its + `start` event as `streamParams`, and the bridge forwards them to the bot as + Twilio `customParameters`. - `Conference` — basic named conferences work, but `waitUrl` hold music has no Bandwidth equivalent, `beep` is only partially supported, and `startConferenceOnEnter`/`endConferenceOnExit`/`maxParticipants` have no diff --git a/src/matrix/twilio-voice.json b/src/matrix/twilio-voice.json index 27c3192..806911e 100644 --- a/src/matrix/twilio-voice.json +++ b/src/matrix/twilio-voice.json @@ -147,7 +147,7 @@ "Stream": { "bxml": "StartStream", "status": "partial", - "notes": "Twilio WS message schema is emulated by the translator's stream bridge; live Bandwidth-side binding requires fixture capture.", + "notes": "Twilio WS message schema is emulated by the translator's stream bridge; live Bandwidth-side binding requires fixture capture. children become nested elements (Bandwidth allows at most 12) and reach the bot as customParameters.", "docsUrl": "https://dev.bandwidth.com/docs/voice/bxml/startStream", "attributes": {} }, diff --git a/src/streams/bridge.ts b/src/streams/bridge.ts index 4b9f1c3..e56d178 100644 --- a/src/streams/bridge.ts +++ b/src/streams/bridge.ts @@ -20,11 +20,39 @@ export interface BridgeOpts { botUrl: string; callSid: string; accountSid: string; - /** Optional key/value pairs forwarded verbatim in the TwiML start message. */ + /** Key/value pairs forwarded verbatim as `customParameters` in the Twilio + * "start" message. These are the TwiML values, which the + * translator emits as and Bandwidth echoes back in its own + * "start" event as `streamParams`; build them with customParametersFromBwStart. */ customParameters?: Record; source: BwStreamSource; } +/** + * Map Bandwidth's StartStream WebSocket "start" event to Twilio `customParameters`. + * + * Bandwidth copies every under the into + * the start event as `streamParams: { name: value, ... }` (a flat map, per the + * StartStream docs). Twilio delivers the same data as `start.customParameters`, + * also a flat string map, so the mapping is a copy with primitive values + * coerced to strings. Nested objects and arrays are not in the documented + * shape and are skipped rather than forwarded as "[object Object]". Anything + * that is not a plain object yields an empty map; a bot always receives a + * `customParameters` object, never undefined. + */ +export function customParametersFromBwStart(event: unknown): Record { + const params = (event as { streamParams?: unknown } | null)?.streamParams; + if (params === null || typeof params !== "object" || Array.isArray(params)) return {}; + // Null prototype so a key literally named "__proto__" is stored as an own + // property instead of hitting the Object.prototype setter and vanishing. + const out: Record = Object.create(null); + for (const [k, v] of Object.entries(params as Record)) { + if (typeof v === "string") out[k] = v; + else if (typeof v === "number" || typeof v === "boolean") out[k] = String(v); + } + return out; +} + export class TwilioStreamBridge { readonly streamSid: string; private ws: WebSocket; diff --git a/src/translator/translate.ts b/src/translator/translate.ts index c551552..5c10ec6 100644 --- a/src/translator/translate.ts +++ b/src/translator/translate.ts @@ -543,8 +543,92 @@ const TWILIO_STREAM_TRACK_TO_BW: Record = { both_tracks: "both", }; +// Bandwidth's documented limits on under : at most +// 12 elements, name up to 256 chars, value up to 2048 chars. Exceeding any of +// them makes Bandwidth reject the whole BXML document, not just the one param, +// so offending s are dropped with a warning instead. Twilio's only +// limit is 500 chars for name+value combined, which does bound value well under +// 2048 but leaves name free to exceed 256 in perfectly valid TwiML. +const MAX_STREAM_PARAMS = 12; +const MAX_STREAM_PARAM_NAME = 256; +const MAX_STREAM_PARAM_VALUE = 2048; + +/** Twilio children → BW . + * Bandwidth copies these into the WebSocket "start" event as a `streamParams` + * map, which the stream bridge forwards to the bot as Twilio `customParameters` + * (see customParametersFromBwStart in streams/bridge.ts). Order is preserved. */ +function streamParams(stream: TwimlNode, findings: Finding[]): XmlEl[] { + const out: XmlEl[] = []; + const seen = new Set(); + let beyondCap = 0; + // Names can be arbitrarily long in TwiML; never echo more than this into a finding. + const brief = (s: string | undefined) => (s === undefined ? "" : s.length > 32 ? s.slice(0, 32) + "…" : s); + for (const child of stream.children) { + if (child.name !== "Parameter") { + warn("Stream", `Stream child <${child.name}> has no Bandwidth equivalent and was dropped.`, findings); + continue; + } + // Cap first, so every past the 12 accepted ones is tallied here + // regardless of whatever else might be wrong with it. + if (out.length >= MAX_STREAM_PARAMS) { + beyondCap++; + continue; + } + const { name, value } = child.attrs; + if (name === undefined || value === undefined) { + warn( + "Stream", + `Stream requires both name and value; dropped .`, + findings, + ); + continue; + } + if (name.length > MAX_STREAM_PARAM_NAME) { + warn( + "Stream", + `StreamParam name exceeds Bandwidth's ${MAX_STREAM_PARAM_NAME}-character limit ` + + `(${name.length}); dropped .`, + findings, + ); + continue; + } + if (value.length > MAX_STREAM_PARAM_VALUE) { + warn( + "Stream", + `StreamParam value exceeds Bandwidth's ${MAX_STREAM_PARAM_VALUE}-character limit ` + + `(${value.length}); dropped .`, + findings, + ); + continue; + } + // Bandwidth delivers streamParams as a flat map (as does Twilio's + // customParameters), so a repeated name can carry only one value and which + // one wins is undocumented. Keep the first, drop the rest, and say so. + if (seen.has(name)) { + warn( + "Stream", + `Duplicate Stream : streamParams is a flat map, so only the ` + + "first value was kept.", + findings, + ); + continue; + } + seen.add(name); + out.push({ name: "StreamParam", attrs: { name, value } }); + } + if (beyondCap > 0) + warn( + "Stream", + `Bandwidth allows at most ${MAX_STREAM_PARAMS} StreamParam per StartStream; ` + + `${beyondCap} Stream element(s) beyond that were dropped.`, + findings, + ); + return out; +} + /** Twilio noun → BW . mode is bidirectional under - * (audio flows both ways) and unidirectional under (a fork). */ + * (audio flows both ways) and unidirectional under (a fork). + * children become nested elements. */ function streamToStartStream( stream: TwimlNode, mode: "bidirectional" | "unidirectional", @@ -559,7 +643,8 @@ function streamToStartStream( mode, tracks: stream.attrs.track ? TWILIO_STREAM_TRACK_TO_BW[stream.attrs.track] ?? "inbound" : "inbound", }; - return [{ name: "StartStream", attrs }]; + // An empty children array still serializes as a self-closing . + return [{ name: "StartStream", attrs, children: streamParams(stream, findings) }]; } // Per-document counter for generated Connect/Stream names. Twilio's diff --git a/test/streams-wire.test.ts b/test/streams-wire.test.ts index 1f0b760..c2aa1e6 100644 --- a/test/streams-wire.test.ts +++ b/test/streams-wire.test.ts @@ -10,7 +10,11 @@ import { describe, it, expect } from "vitest"; import { WebSocketServer, WebSocket } from "ws"; import { EventEmitter } from "node:events"; -import { TwilioStreamBridge, type BwStreamSource } from "../src/streams/bridge.js"; +import { + TwilioStreamBridge, + customParametersFromBwStart, + type BwStreamSource, +} from "../src/streams/bridge.js"; // ─── helpers ──────────────────────────────────────────────────────────────── @@ -107,6 +111,63 @@ describe("start message", () => { }); }); + // VAPI-3989: Bandwidth echoes values in its "start" event as a + // flat `streamParams` map; the bot must see them as Twilio customParameters. + it("forwards Bandwidth streamParams to the bot as customParameters", async () => { + // Shape per the StartStream docs' start-event example. + const bwStart = { + eventType: "start", + metadata: { accountId: "9900778", callId: "c-abc", to: "+15550001111", from: "+15550002222" }, + streamParams: { callSid: "CA123", tenant: "acme" }, + }; + const port = nextPort(); + const { messages, close } = await botServer(port); + const source = new FakeBwSource(); + const bridge = new TwilioStreamBridge({ + botUrl: `ws://127.0.0.1:${port}`, + callSid: "CA123", + accountSid: "AC222", + customParameters: customParametersFromBwStart(bwStart), + source, + }); + await bridge.ready(); + await waitFor(() => messages.length >= 2); + bridge.close(); + close(); + + const startMsg = messages.find((m: any) => m.event === "start") as any; + expect(startMsg.start.customParameters).toEqual({ callSid: "CA123", tenant: "acme" }); + }); + + it("customParametersFromBwStart tolerates missing or malformed streamParams", () => { + expect(customParametersFromBwStart({ eventType: "start" })).toEqual({}); + expect(customParametersFromBwStart({ streamParams: null })).toEqual({}); + expect(customParametersFromBwStart({ streamParams: [1, 2] })).toEqual({}); + expect(customParametersFromBwStart(undefined)).toEqual({}); + expect(customParametersFromBwStart("start")).toEqual({}); + // Values are always strings on the Twilio side, even if Bandwidth ever sent a number. + expect(customParametersFromBwStart({ streamParams: { n: 42, b: true, s: "x", nil: null } })).toEqual({ + n: "42", + b: "true", + s: "x", + }); + // The documented shape is flat; nested values are skipped, not forwarded as "[object Object]". + expect(customParametersFromBwStart({ streamParams: { o: { a: 1 }, arr: [1], s: "x" } })).toEqual({ s: "x" }); + }); + + it("customParametersFromBwStart keeps a parameter literally named __proto__", () => { + // JSON.parse yields an own "__proto__" key; a plain {} target would route the + // assignment to the Object.prototype setter and silently lose the pair. + const evt = JSON.parse('{"streamParams":{"__proto__":"p","a":"1"}}'); + const out = customParametersFromBwStart(evt); + expect(Object.keys(out).sort()).toEqual(["__proto__", "a"]); + expect(Object.getOwnPropertyDescriptor(out, "__proto__")?.value).toBe("p"); + // Survives the wire: the bot sees both keys. + expect(JSON.stringify(out)).toBe('{"__proto__":"p","a":"1"}'); + // And nothing leaked onto the global prototype. + expect(({} as any).p).toBeUndefined(); + }); + it("customParameters defaults to empty object when omitted", async () => { const port = nextPort(); const { messages, close } = await botServer(port); diff --git a/test/translate-stream-conference.test.ts b/test/translate-stream-conference.test.ts index a342113..e500796 100644 --- a/test/translate-stream-conference.test.ts +++ b/test/translate-stream-conference.test.ts @@ -53,6 +53,154 @@ describe("Stream lifecycle", () => { expect(r.bxml).not.toMatch(/]*\/><\/Response>/); }); + // VAPI-3989: children used to be dropped silently, so bots got an + // empty customParameters map and could not identify the call or tenant. + describe("Stream → StreamParam (VAPI-3989)", () => { + it("emits one nested StreamParam per Parameter, in order, inside StartStream", () => { + const r = translateTwiml( + ` + + + `, + { rewriteUrl: rw }, + ); + expect(r.hasErrors).toBe(false); + expect(r.bxml).toMatch( + /]*name="agent"[^>]*><\/StartStream>/, + ); + // No drop warnings when every Parameter is valid and within the limit. + expect(r.findings.some((f) => /dropped/.test(f.message))).toBe(false); + }); + + it("output differs from the same Stream without Parameters", () => { + const a = translateTwiml(``); + const b = translateTwiml( + ``, + ); + expect(b.bxml).not.toBe(a.bxml); + expect(a.bxml).not.toContain("StreamParam"); + }); + + it("also applies to Start>Stream forks", () => { + const r = translateTwiml( + ``, + ); + expect(r.hasErrors).toBe(false); + expect(r.bxml).toContain(``); + expect(r.bxml).not.toContain(" { + const r = translateTwiml( + ``, + ); + expect(r.bxml).toContain(``); + }); + + it("keeps the first 12 Parameters and warns about the rest (Bandwidth limit)", () => { + const params = Array.from({ length: 14 }, (_, i) => ``).join(""); + const r = translateTwiml( + `${params}`, + ); + expect(r.hasErrors).toBe(false); + expect(r.bxml.match(/ f.verb === "Stream" && /at most 12 StreamParam/.test(f.message)); + expect(capWarnings).toHaveLength(1); + expect(capWarnings[0].message).toContain("2 Stream element(s) beyond that were dropped"); + }); + + it("tallies everything past the 12th accepted Parameter under the cap, even if also invalid", () => { + const valid = Array.from({ length: 12 }, (_, i) => ``).join(""); + const r = translateTwiml( + `${valid}`, + ); + expect(r.bxml.match(/ /1 Stream element\(s\) beyond that were dropped/.test(f.message))).toBe(true); + expect(r.findings.some((f) => /name exceeds/.test(f.message))).toBe(false); + }); + + it("keeps the first of duplicate Parameter names and warns, since streamParams is a flat map", () => { + const r = translateTwiml( + ` + + + + `, + ); + expect(r.hasErrors).toBe(false); + expect(r.bxml.match(/`); + expect(r.bxml).not.toContain(`value="second"`); + expect(r.findings.filter((f) => f.verb === "Stream" && /Duplicate Stream /.test(f.message))).toHaveLength(1); + }); + + it("never echoes more than 32 characters of a name into a finding", () => { + const huge = "x".repeat(5000); + const r = translateTwiml( + ``, + ); + const f = r.findings.find((f) => /requires both name and value/.test(f.message))!; + expect(f).toBeDefined(); + expect(f.message.length).toBeLessThan(200); + expect(f.message).toContain(`name="${"x".repeat(32)}…"`); + }); + + it("drops a Parameter whose name exceeds Bandwidth's 256-character limit", () => { + // Valid TwiML: Twilio's only limit is 500 chars for name+value combined. + // Bandwidth would reject the entire BXML document for this one name. + const longName = "n".repeat(300); + const r = translateTwiml( + ` + + + `, + ); + expect(r.hasErrors).toBe(false); + expect(r.bxml).not.toContain(longName); + expect(r.bxml).toContain(``); + expect(r.findings.some((f) => f.verb === "Stream" && /name exceeds Bandwidth's 256-character limit \(300\)/.test(f.message))).toBe(true); + }); + + it("keeps a 256-character name and drops a value over 2048 characters", () => { + const maxName = "n".repeat(256); + const longValue = "v".repeat(2049); + const r = translateTwiml( + ` + + + `, + ); + expect(r.bxml).toContain(``); + expect(r.bxml).not.toContain(longValue); + expect(r.findings.some((f) => f.verb === "Stream" && /value exceeds Bandwidth's 2048-character limit \(2049\)/.test(f.message))).toBe(true); + }); + + it("drops a Parameter missing name or value with a warning instead of emitting invalid BXML", () => { + const r = translateTwiml( + ` + + + + `, + ); + expect(r.hasErrors).toBe(false); + expect(r.bxml.match(/`); + expect(r.findings.filter((f) => f.verb === "Stream" && /requires both name and value/.test(f.message))).toHaveLength(2); + }); + + it("warns about non-Parameter children of Stream", () => { + const r = translateTwiml( + ``, + ); + expect(r.hasErrors).toBe(false); + expect(r.bxml).not.toContain("Bogus"); + expect(r.findings.some((f) => f.verb === "Stream" && //.test(f.message))).toBe(true); + }); + }); + it("Start>Stream → StartStream mode=unidirectional (fork)", () => { const r = translateTwiml( ``,