feat: add call escalation tooling for Telnyx and Twilio telephony bac…

[jjleng]

…kends, including phone number and SIP URI validation, hold state guards, and deterministic transfer bridging.

[jjleng]

@claude

[claude]

Claude finished @jjleng's task in 2m 40s —— View job

---

Code Review

• Read CLAUDE.md for repo guidelines (none found)
• Analyze PR diff and changed files
• Review transport layer changes (Telnyx/Twilio)
• Review agent-kit and swarm changes
• Review server and reactor changes
• Review API and frontend changes
• Post comprehensive review feedback

---

This is a substantial, well-architected feature. The supervised-conference transfer design (deferred hold → webhook bridge → TransferResult event loop) is correct and avoids the common pitfall of dropping the original caller before the destination answers. Good work overall. Issues below are roughly ordered by severity.

---

Bugs

1. Native Gemini path resolves destination_name, not phone number

File: voice/engine/crates/agent-kit/src/agent_backends/native.rs:366-380

let destination = args_json.get("destination_name").and_then(...).to_string();
return Some(NativeAgentEvent::EscalateCall { destination, reason });

destination here is the raw label ("Support Team"), not the E.164 number. The DefaultAgentBackend path correctly resolves the name via escalation_destinations, but NativeMultimodalBackend does not — it has access to self.tool_schemas but no escalation_destinations list. The TransportCommand::Transfer that eventually fires will contain the human-readable name, causing the Telnyx/Twilio API call to fail.

The fix is to pass escalation_destinations into NativeMultimodalBackend and resolve the name there, mirroring handle_escalate_call in default.rs.

Fix this →

---

2. No timeout when stuck in holding state

Files: voice/engine/src/reactor/mod.rs, voice/engine/src/native_session.rs

If the transfer webhook never fires (network partition, provider outage, webhook misconfiguration), the reactor/native session will stay in holding state indefinitely, generating a 440 Hz tone forever. There is no watchdog timer to terminate the session after N seconds.

Recommend adding a timer (e.g. 60s) that fires TransferResult { succeeded: false, reason: Some("transfer timeout") } back into control_rx.

---

3. completed state silently removes waiter without sending TransferResult

File: voice/engine/src/server.rs — telephony_twilio_transfer_status_handler

"completed" => {
    // The outbound leg completed ... remove the waiter silently.
    drop(waiter);
    state.transfer_waiters.remove(&original_call_sid);
}

If Twilio sends completed without a prior in-progress (e.g. race where the destination answers and immediately hangs up before the bridge webhook is processed), the session stays in holding forever. A TransferResult { succeeded: false, reason: "completed-before-bridge" } should be sent here, or the waiter must be checked for whether a bridge already happened.

---

Security

4. Conference name not XML-escaped in Twilio TwiML

File: voice/engine/crates/transport/src/telephony/providers/twilio.rs:135,182

let twiml = format!(
    r#"...<Conference endConferenceOnExit="true">{}</Conference>..."#,
    conference_name   // ← not XML-escaped
);

In practice conference_name is always feros-transfer-{call_sid} (alphanumeric), so this is not currently exploitable. But it's a latent XSS/injection if the conference naming ever changes. Use proper XML escaping (&, <, etc.) or a TwiML builder.

---

5. Telnyx webhook endpoint has no signature validation

File: voice/engine/src/server.rs — telephony_telnyx_transfer_status_handler

The Twilio handler validates x-twilio-signature (HMAC-SHA1). The Telnyx handler accepts any JSON POST unconditionally. Telnyx uses Ed25519 webhook signatures (telnyx-signature-ed25519 / telnyx-timestamp headers). Leaving this unauthenticated allows any external actor to spoof call.answered events and bridge arbitrary callers into conferences.

---

6. Telnyx conference name is URL-injected without encoding

File: voice/engine/crates/transport/src/telephony/providers/telnyx.rs:~180

let filter_url = format!("https://api.telnyx.com/v2/conferences?filter[name]={}", conference_id_or_name);

conference_id_or_name is not percent-encoded. While currently safe (names are always feros-transfer-{uuid}), it should use urlencoding::encode for correctness.

---

Code Quality

7. reqwest::Client::new() created on every API call

Files: telnyx.rs, twilio.rs

Both providers create a fresh reqwest::Client for every REST call. This bypasses connection pooling and adds TCP + TLS handshake overhead on every transfer. The client should be stored on the provider struct and reused.

---

8. Silent unwrap_or_default() on API response deserialization

Files: telnyx.rs, twilio.rs (multiple places)

let json: serde_json::Value = serde_json::from_str(&body).unwrap_or_default();
let new_call_id = json.pointer("/data/call_control_id")
    .and_then(|v| v.as_str())
    .unwrap_or("")
    .to_string();

If the provider returns unexpected JSON, the call SID silently becomes "". The caller has no way to distinguish "API returned unexpected shape" from "API returned an empty SID". At minimum log the raw body when the pointer lookup returns None.

---

9. Frontend: useEffect array dependency causes potential infinite loop

File: studio/web/src/components/agent/agent-config-editor.tsx:291

}, [fields?.language, fields?.timezone, fields?.voice_id, fields?.gemini_live_model, fields?.escalation_destinations]);

fields?.escalation_destinations is an array. If the parent re-creates this array on each render (e.g. via JSON.parse), the reference changes every render, causing this effect to re-run infinitely. Use JSON.stringify(fields?.escalation_destinations) as the dependency, or useMemo to stabilize the reference.

---

10. Frontend: + Add Destination immediately saves empty records

File: agent-config-editor.tsx:847

onClick={() => {
    const newDests = [...escalationDestinations, { id: ..., name: "", phone_number: "" }];
    setEscalationDestinations(newDests);
    patchField({ escalation_destinations: newDests.map(...) }); // saves empty entry immediately
}}

This persists { name: "", phone_number: "" } to the DB while the user is still typing. Save should be deferred to the onBlur of the fields or a dedicated save button.

---

11. No server-side validation that transfer destination is in the allowed list

File: voice/engine/crates/agent-kit/src/agent_backends/default.rs:700

.unwrap_or_else(|| {
    warn!("[agent_backend] escalate_call: unknown destination '{}' — using name as raw number", destination_name);
    destination_name.to_string()
});

The LLM schema enforces the enum via "enum": dest_names, which should prevent unknown names in practice. However, the fallback silently uses the raw string as a phone number. A hard reject (return early without setting pending_escalate_call) would be safer than a warning + forwarding to an arbitrary string.

---

Nits

• voice/engine/crates/transport/src/telephony/config.rs has a stray double blank line added at the bottom of impl Default.
• voice/engine/crates/agent-kit/src/agent_backends/default.rs:183: docstring says /// A \escalate_call`— should be/// An`.
• voice/engine/crates/agent-kit/src/agent_backends/native.rs:248: push_transfer_failure_result is a pub method that crosses module boundaries; it should be pub(crate) since it's internal to the engine.

---

Summary

Severity	Count
Bug	3
Security	3
Quality	5
Nit	3

The most important fixes before merging are: #1 (native path sends label instead of phone number), #2 (holding timeout), and #5 (Telnyx webhook unauthenticated).