Skip to content

VAPI-4042: emit Twilio's Gather/Record defaults and clamp to BW's ranges - #32

Open
mramasubramanian-bw wants to merge 3 commits into
mainfrom
VAPI-4042/gather-record-defaults
Open

mramasubramanian-bw wants to merge 3 commits into
mainfrom
VAPI-4042/gather-record-defaults

Conversation

@mramasubramanian-bw

@mramasubramanian-bw mramasubramanian-bw commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • <Gather>/<Record> now use Twilio's own default values instead of letting Bandwidth's different defaults silently change IVR behavior: finishOnKey, timeout → firstDigitTimeout/interDigitTimeout, Record's maxLength, and Record's timeout → silenceTimeout (this wasn't translated to anything before).
  • numDigits/timeout/maxLength are now kept within the limits Bandwidth documents, with a warning, instead of letting an out-of-range value through for Bandwidth to reject. A value that isn't a plain number at all (empty, "abc", a hex or scientific-notation string) now warns and falls back to Twilio's own default, instead of being copied into the BXML as written.
  • Record's finishOnKey is cut down to the one real keypad key (a digit, *, or #) Bandwidth's Record can use to stop — preferring #, then *, then the first valid one present — since Bandwidth can't use a whole set of keys the way Twilio does. A value with no valid key at all (e.g. a letter) is dropped rather than passed through.
  • A <Gather>/<Record> with no action now sends its result back to the same page that served it, matching Twilio, when the server knows that page's address.
  • playBeep/trim now warn when they're left out too (Twilio's default plays a beep and trims silence), not only when someone sets them to a different value on purpose.

Motivation / Context

translateGather/translateRecord only copied an attribute when the TwiML set it. Where Twilio's default is different from Bandwidth's, the translated verb behaved differently with no warning: a default <Record> cut voicemail off at 60 seconds with no way to stop on silence, a default <Gather> counted # as just another digit and could cut an entry short mid-way, and a value outside Bandwidth's limits made Bandwidth reject the whole call. Tracked in VAPI-4042.

Reviewer Notes

  • "Send the result back to the same page" is driven by a new TranslateOptions.documentUrl, set only by the server (one line in app.ts, next to the existing rewriteUrl). The standalone BXML generator (src/bxml-generator) leaves it unset and keeps the original warning, since it has no live server to send a request back to — doing this there would just produce a useless empty gatherUrl="" in a static file.
  • Gather's timeout sets both firstDigitTimeout and interDigitTimeout, each kept within its own limits (0-60 and 1-60). One Twilio value can land differently for each — e.g. timeout="0" leaves firstDigitTimeout="0" alone but raises interDigitTimeout to 1 — so these are two separate warnings, not one.
  • Record's finishOnKey: Bandwidth's docs say "this digit" (singular), so a Twilio value with more than one character (including the implicit default, any of 1234567890*#) can only be reduced to one character — and only to a character Bandwidth actually recognizes as a key (a digit, *, or #). A single such character someone set on purpose passes through with no warning, since that's an exact match.
  • playBeep/trim warn whenever Twilio's own behavior (play a beep / trim silence) wouldn't happen on Bandwidth — including when they're left out, since Twilio's defaults for both are "yes, do it" (playBeep defaults to true, trim defaults to trim-silence). The one setting that doesn't warn for each (playBeep="false", trim="do-not-trim") is the one case where Bandwidth's fixed behavior (never beeps, never trims) actually matches.
  • Record timeout="0" (Twilio: turn off silence detection) and Bandwidth's own silenceTimeout="0" (no timeout) already mean the same thing, so that value needed no special handling.
  • timeout/maxLength set to an empty string (timeout="") is treated the same as leaving the attribute out, not as the number zero — an empty string isn't a value a customer would set on purpose, so there's no warning either, just the same default used for the missing case. An empty finishOnKey is handled differently, since both Twilio and Bandwidth treat that specific value as "turn the stop-key feature off" on purpose.
  • Warnings about an out-of-range or invalid value name the attribute the customer actually wrote (timeout, maxLength), not only Bandwidth's name for it (firstDigitTimeout, maxDuration), while still saying which Bandwidth attribute is affected — so the warning can be found by searching the customer's own TwiML.
  • Identical findings (same verb, same message) are now reported once per document rather than once per verb — a document with many ordinary bare <Record>s previously repeated the same 3 generic warnings for every one of them, which could bury a real out-of-range warning under dozens of repeats of an unrelated one.
  • The duplicated "no action → send the result back to the same page, or warn if that's not possible" logic in translateGather and translateRecord is now one shared helper.

Breaking Changes

  • A bare <Gather> (no finishOnKey) now stops on #, matching Twilio; before, # was just another digit (Bandwidth's own default turns off the stop-key feature). Any flow that expected # to be collected along with the other digits will now stop there instead.
  • A bare <Record> (no timeout) now stops after 5 seconds of silence (silenceTimeout), which had no equivalent at all before — a long pause mid-recording can now end it sooner than it used to.
  • A <Gather>/<Record> with no action attribute now sends a real request back to the customer's current page (carrying Digits/RecordingUrl, etc.), instead of Bandwidth quietly dropping the result (Gather) or moving on to the next verb once the recording ends (Record). This matches Twilio, but it's a request shape that page may not have expected before.
  • numDigits/timeout/maxLength outside Bandwidth's documented range, or not a plain number at all, are now brought back into range (with a warning) instead of passed through as written, which used to risk Bandwidth rejecting the call outright.
  • A bare <Gather> with no timeout set does not change behavior: Bandwidth already defaults firstDigitTimeout/interDigitTimeout to 5 seconds on its own, matching Twilio. What does change: an explicit timeout previously only reached firstDigitTimeout, silently leaving interDigitTimeout at Bandwidth's 5-second default regardless of what was set — now it governs both, matching Twilio's single timeout value doing both jobs. (An earlier version of this PR description got this backwards; a reviewer caught it.)

Tests

Added

Test Assertion
Gather > Twilio defaults › emits finishOnKey's Twilio default (#) when omitted terminatingDigits="#" when finishOnKey isn't set
… › passes finishOnKey="" through unchanged Twilio's and Bandwidth's "off" value is left as-is
… › emits timeout's Twilio default (5) as both firstDigitTimeout and interDigitTimeout A left-out timeout still sets both attributes
… › does not emit maxDigits when numDigits is omitted Bandwidth's own default (50) is left alone
Gather > keeping values within Bandwidth's limits › brings an out-of-range timeout back within limits for both firstDigitTimeout and interDigitTimeout, with a warning for each One Twilio value produces two separate warnings
… › enforces interDigitTimeout's minimum (1) separately from firstDigitTimeout's minimum (0) timeout="0" only raises one of the two attributes
… › limits numDigits to Bandwidth's 50-digit maximum An out-of-range numDigits is brought into range with a warning
… › leaves an in-range value untouched with no warning No warning fires for ordinary values
… › treats timeout="" the same as omitted, not as 0 Number("") is 0; an empty value must not be read as a real zero
… › normalizes a non-decimal but in-range value instead of passing it through as written timeout="0x3C" becomes "60", not left as the hex string
… › normalizes scientific notation to a plain decimal numDigits="1e1" becomes "10"
… › falls back to Bandwidth's own default and warns when the value isn't a number at all numDigits="abc" becomes "50" with a warning, not passed through as "abc"
Gather > no action, re-requesting the current document › points gatherUrl at the current document when the server provides one, with no warning documentUrl set → gatherUrl points back at the same page; the Gather's position and empty-result flag still travel through
… › still warns when no documentUrl is given Standalone generation keeps the original warning
Record > Twilio defaults › emits maxLength's Twilio default (3600) when omitted maxDuration="3600", not Bandwidth's 60-second default
… › emits timeout as silenceTimeout, defaulting to Twilio's 5 seconds silenceTimeout is translated to something at all now
… › passes timeout="0" through unchanged Twilio's "off" value matches Bandwidth's
Record > keeping values within Bandwidth's limits › limits maxLength to Bandwidth's 10800s (3h) maximum Warning names the original value
… › limits timeout (silenceTimeout) to Bandwidth's 10800s maximum Same, for silenceTimeout
… › treats maxLength="" the same as omitted, not as 0 An empty maxLength must not cut recordings to 1 second
… › treats timeout="" the same as omitted, not as 0 Same, for Record's timeout
Record > finishOnKey -> a single terminatingDigit › warns when omitted Bandwidth's own # default only covers one of Twilio's "any key" set
… › passes a single explicit digit through with no warning Exact match, nothing to reduce
… › prefers # when a multi-character set includes it, and warns # is picked over the other characters in the set
… › falls back to the first character when # is not in a multi-character set, and warns Always picks the same character when # isn't an option
… › picks the first valid character in a mixed set rather than blindly the first character finishOnKey="ab1" picks "1", not "a"
… › drops the attribute and warns when there is no valid key at all, rather than emitting an invalid one finishOnKey="a" leaves terminatingDigits unset instead of emitting "a"
Record > playBeep and trim › warns when playBeep is omitted and when explicitly true Twilio's default (a beep plays) isn't matched
… › does not warn about playBeep when explicitly false The one setting Bandwidth's behavior matches
… › warns when trim is omitted and when explicitly trim-silence Twilio's default (trims silence) isn't matched
… › does not warn about trim when explicitly do-not-trim The one setting Bandwidth's behavior matches
Record > no action, re-requesting the current document › points recordCompleteUrl at the current document when the server provides one recordCompleteUrl points back at the same page, no "missing action" warning
… › warns when no documentUrl is given Standalone generation keeps the original warning
repeated identical findings collapse to one › a document with many bare Records reports each generic warning once, not once per Record 10 bare <Record>s produce one playBeep warning, not ten
… › does not collapse findings that differ, e.g. out-of-range warnings naming different values Two <Record>s with different out-of-range maxLengths each keep their own warning
Gather/Record with no action re-request the current document, like Twilio › Gather posts Digits back to the same voice URL that served it Through the real server end to end: digits go back to the original voice page
… › Record posts RecordingUrl back to the same voice URL that served it Same, for a finished recording
generate → band BXML conformance › band accepts generated BXML for gather-record-limits.xml A new fixture covering every limit and the finishOnKey=""/multi-character cases, checked against Bandwidth's own CLI

Modified

Test Assertion Why
Gather › maps attributes and nested prompts Also checks interDigitTimeout="7" timeout now sets both attributes, not just firstDigitTimeout

Test Plan

  • npm run typecheck
  • npm test, including test/bxml-generator-band-conformance.test.ts with the band CLI installed
  • Ran the real translator server with hand-made Bandwidth requests (no phone): bare Gather/Record defaults, extreme/invalid out-of-range values (empty strings, non-numeric, hex, scientific notation), an invalid finishOnKey, and the no-action case sending results back to the original voice page; band bxml raw accepted every document
  • Confirmed each fix in this PR against a reproduction that failed without it (empty-string, non-numeric, and invalid-finishOnKey handling; the attribute-name mix-up in warnings; the repeated-findings issue)
  • Live call: a bare Gather with no finishOnKey stops on #, and the # is not included in Digits
  • Live call: a bare Record with no timeout stops on silence (silenceTimeout), not at the maxLength safety cap
  • Live call: a bare Gather/Record with no action sends its result back to the page that served it, with the caller's digits/recording

Twilio and Bandwidth default differently for several Gather/Record
attributes; leaving them unset let Bandwidth's own default silently
change IVR behavior instead of matching Twilio's.

Gather:
- finishOnKey omitted -> emit "#" (Twilio's default); Bandwidth's own
  default is "" (disabled). finishOnKey="" still passes through.
- timeout omitted -> default "5", set as both firstDigitTimeout and
  interDigitTimeout (Bandwidth splits what Twilio combines).
- numDigits/timeout clamped to Bandwidth's documented ranges with a
  warning naming the original value.
- A Gather with no action re-requests the current document, matching
  Twilio, when the server provides the document's URL.

Record:
- maxLength omitted -> emit "3600" (Twilio's default); Bandwidth's own
  default is 60s.
- timeout (silence ends the recording) is now mapped at all, to
  silenceTimeout; previously dropped entirely. timeout="0" (Twilio:
  disable) and silenceTimeout="0" (Bandwidth: no timeout) agree.
- finishOnKey narrowed to Bandwidth's single terminating digit
  (preferring #, then *, then the first character), with a warning,
  since Bandwidth can't represent Twilio's multi-key set (default: any
  key). A single explicit digit passes through unchanged.
- playBeep/trim now warn on the omitted (Twilio-default) case too, not
  only when explicitly set to a non-matching value.
- A Record with no action re-requests the current document, matching
  Twilio, when the server provides the document's URL.

The "re-request current document" behavior is driven by a new
TranslateOptions.documentUrl, set only by the server (one line in
app.ts); standalone BXML generation leaves it unset and keeps the
original warning, since there's no live document for it to re-fetch.
@mramasubramanian-bw
mramasubramanian-bw requested review from a team as code owners October 13, 2026 15:14
@bwappsec

bwappsec commented Oct 13, 2026 •

Copy link
Copy Markdown

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues
✅ Licenses 0 0 0 0 0 issues
✅ Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

The warning a customer sees when a value is outside Bandwidth's range
said "clamped to X" instead of plainly saying what happened. It now
says "using X instead". The matrix notes, AGENTS.md, and the new test
names follow the same change ("kept within Bandwidth's limits" /
"limits X to Bandwidth's maximum" rather than "clamped").

The clampAttr function name and its internal comments are unchanged;
"clamp" was already an internal term in this file (the Say/Play loop
count logic), so this only touches text a person actually reads.
- "" on timeout/maxLength was read as 0 (Number("") === 0), not as
  missing: <Record maxLength=""/> cut voicemail to 1 second, and
  <Gather timeout=""/> sent an empty firstDigitTimeout. Treated as
  missing now (clampAttr's callers use || instead of ?? for these).
- A value that isn't a plain finite number ("abc", "Infinity") now
  warns and falls back to Twilio's default, instead of being copied
  into the BXML as-is for Bandwidth to reject. A value that IS a
  number but an unusual form ("0x3C", "1e1") is normalized to a plain
  decimal instead of being emitted as written.
- Record's finishOnKey narrowing now only picks a real keypad
  character: "ab1" narrows to "1", not "a", and a value with no digit,
  "*", or "#" at all (e.g. "a") is dropped instead of passed through,
  so Bandwidth's own default applies.
- Range warnings now name the Twilio attribute the customer actually
  wrote (timeout, maxLength) rather than only Bandwidth's name for it
  (firstDigitTimeout, maxDuration), while still saying which BXML
  attribute is affected.
- Identical findings (verb + message) collapse to one per document, so
  a document with many ordinary bare Records doesn't bury a real
  out-of-range warning under repeats of the same generic one.
- Corrected AGENTS.md: Bandwidth already defaults firstDigitTimeout and
  interDigitTimeout to 5s on its own, so an omitted Gather timeout
  isn't a behavior change; the real fix is that an explicit timeout
  previously only reached firstDigitTimeout. Also listed maxLength
  among the range-limited attributes and stopped calling finishOnKey's
  narrowing a range limit.
- Pulled the duplicated "no action -> re-request the current document,
  else warn" branching out of translateGather/translateRecord into one
  resolveActionUrl helper.

This branch has not been deployed

No deployments
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