refactor(voice): tighten WhatsApp call pipeline and broadcast hyphenated terminate status

app/models/channel/whatsapp.rb

@@ -40,9 +40,7 @@ class Channel::Whatsapp < ApplicationRecord
     'Whatsapp'
   end
 
-  # Mirrors Channel::TwilioSms#voice_enabled? so the call subsystem can
-  # duck-type across providers. Backed by the JSON config rather than a column
-  # because all other WhatsApp Cloud capability flags live in `provider_config`.
+  # Mirrors Channel::TwilioSms#voice_enabled? so the call subsystem can duck-type across providers.
   def voice_enabled?
     provider_config['calling_enabled'].present?
   end

enterprise/app/builders/enterprise/messages/message_builder.rb

@@ -7,9 +7,6 @@ module Enterprise::Messages::MessageBuilder
     super
   end
 
-  # Voice-capable channels (Twilio voice, WhatsApp Cloud Calling) all expose
-  # `voice_enabled?`; treat any of them as eligible for the incoming voice_call
-  # bubble bypass.
   def voice_call_inbox?
     @conversation.inbox.channel.try(:voice_enabled?)
   end

enterprise/app/jobs/enterprise/webhooks/whatsapp_events_job.rb

@@ -8,9 +8,7 @@ module Enterprise::Webhooks::WhatsappEventsJob
 
   private
 
-  # Call webhooks don't share the message-mutex sender_id; we lock per-call_id
-  # inside `handle_call_events` instead so multi-call batches don't share a
-  # single lock keyed on the first call's id.
+  # Lock per-call_id inside handle_call_events instead of the parent's per-sender mutex.
   def contact_sender_id(params)
     return nil if call_event?(params)
 
@@ -25,9 +23,7 @@ module Enterprise::Webhooks::WhatsappEventsJob
     params.dig(:entry, 0, :changes, 0, :value, :messages, 0, :interactive, :type) == 'call_permission_reply'
   end
 
-  # Acquire a per-call_id mutex around each call payload so that connect /
-  # terminate webhooks for the same call are serialized — even when Meta
-  # batches multiple calls in one webhook envelope.
+  # Per-call_id mutex so connect/terminate for the same call serialize across batches.
   def handle_call_events(channel, params)
     calls = params.dig(:entry, 0, :changes, 0, :value, :calls) || []
     calls.each do |call_payload|

enterprise/app/models/call.rb

@@ -39,6 +39,8 @@ class Call < ApplicationRecord
   # Frontend voice bubbles/stores expect inbound/outbound string values
   DISPLAY_DIRECTION = { 'incoming' => 'inbound', 'outgoing' => 'outbound' }.freeze
 
+  DEFAULT_STUN_URL = 'stun:stun.l.google.com:19302'.freeze
+
   enum :provider, { twilio: 0, whatsapp: 1 }
   enum :direction, { incoming: 0, outgoing: 1 }
 
@@ -69,7 +71,7 @@ class Call < ApplicationRecord
 
   # Browser ↔ Meta WebRTC needs at least one STUN server to discover its public srflx candidate.
   def self.default_ice_servers
-    urls = ENV.fetch('VOICE_CALL_STUN_URLS', 'stun:stun.l.google.com:19302').split(',').map(&:strip).reject(&:blank?)
+    urls = ENV.fetch('VOICE_CALL_STUN_URLS', DEFAULT_STUN_URL).split(',').filter_map { |u| u.strip.presence }
     [{ urls: urls }]
   end
 

enterprise/app/services/voice/call_message_builder.rb

@@ -15,13 +15,13 @@ class Voice::CallMessageBuilder
     message = call.message
     return unless message
 
-    data = (message.content_attributes || {}).deep_dup
-    data['data'] ||= {}
-    data['data']['status'] = status.to_s.tr('_', '-') if status
-    data['data']['accepted_by'] = { 'id' => agent.id, 'name' => agent.name } if agent
-    data['data']['duration_seconds'] = duration_seconds if duration_seconds
+    patch = {
+      'status' => status&.to_s&.tr('_', '-'),
+      'accepted_by' => agent && { 'id' => agent.id, 'name' => agent.name },
+      'duration_seconds' => duration_seconds
+    }.compact
 
-    message.update!(content_attributes: data)
+    message.update!(content_attributes: (message.content_attributes || {}).deep_merge('data' => patch))
     message
   end
 
@@ -43,8 +43,7 @@ class Voice::CallMessageBuilder
     call.outgoing? ? call.accepted_by_agent : call.contact
   end
 
-  # `call_source` lets the FE disambiguate WhatsApp vs Twilio for UI copy and
-  # event routing without fetching the whole Call record client-side.
+  # call_source lets the FE disambiguate WhatsApp vs Twilio without re-fetching the Call.
   def build_data_payload
     {
       'call_id' => call.id,

enterprise/app/services/voice/inbound_call_builder.rb

@@ -1,9 +1,6 @@
 class Voice::InboundCallBuilder
   attr_reader :inbox, :from_number, :call_sid, :provider, :extra_meta
 
-  # `provider` defaults to :twilio for back-compat with the original Twilio-only
-  # call site; WhatsApp passes :whatsapp + extra_meta carrying the SDP offer
-  # and ICE servers.
   def self.perform!(inbox:, from_number:, call_sid:, provider: :twilio, extra_meta: {})
     new(inbox: inbox, from_number: from_number, call_sid: call_sid,
         provider: provider, extra_meta: extra_meta).perform!
@@ -61,8 +58,7 @@ class Voice::InboundCallBuilder
     end
   end
 
-  # WhatsApp ContactInbox.source_id must be digits-only (the wa_id); Twilio
-  # accepts the `+`-prefixed phone number as-is.
+  # WhatsApp ContactInbox.source_id must be digits-only (the wa_id); Twilio accepts the +.
   def source_id_for_provider
     provider == :whatsapp ? from_number.to_s.delete_prefix('+') : from_number
   end

enterprise/app/services/whatsapp/call_permission_reply_service.rb

@@ -35,9 +35,7 @@ class Whatsapp::CallPermissionReplyService
          .first&.contact
   end
 
-  # Pick the conversation that actually requested permission, not just any open
-  # one — a contact with multiple open threads in the same inbox would otherwise
-  # have the wrong conversation cleared and broadcast.
+  # Filter to threads that actually requested permission; multiple open threads otherwise hit the wrong one.
   def find_active_conversation(contact)
     inbox.conversations
          .where(contact: contact)

enterprise/app/services/whatsapp/incoming_call_service.rb

@@ -19,18 +19,15 @@ class Whatsapp::IncomingCallService
 
   def handle_connect(payload)
     call = Call.whatsapp.find_by(provider_call_id: payload[:id])
-    return handle_inbound_connect(payload) if call.nil?
-    return handle_outbound_connect(call, payload) if call.outgoing?
+    return create_inbound_call(payload) if call.nil?
+    return accept_outbound_call(call, payload) if call.outgoing?
 
     Rails.logger.info "[WHATSAPP CALL] Duplicate inbound connect for #{payload[:id]}; ignoring"
   rescue ActiveRecord::RecordNotUnique
     Rails.logger.warn "[WHATSAPP CALL] Duplicate provider_call_id received: #{payload[:id]}"
   end
 
-  # Inbound delegates to Voice::InboundCallBuilder for contact + conversation +
-  # call + message creation; auto-assignment falls out of the standard
-  # Conversation lifecycle. We just stash Meta's SDP offer in meta.
-  def handle_inbound_connect(payload)
+  def create_inbound_call(payload)
     sdp_offer = payload.dig(:session, :sdp)
     call = Voice::InboundCallBuilder.perform!(
       inbox: inbox, from_number: "+#{payload[:from]}", call_sid: payload[:id],
@@ -41,13 +38,10 @@ class Whatsapp::IncomingCallService
     broadcast_incoming(call, sdp_offer)
   end
 
-  def handle_outbound_connect(call, payload)
-    # `in_progress?` skips duplicate connect deliveries; `terminal?` stops a
-    # delayed connect from reopening an already-ended call.
+  def accept_outbound_call(call, payload)
     return if call.in_progress? || call.terminal?
 
-    # Browsers always emit a=setup:active in answers, but Meta sometimes echoes
-    # actpass; pin it to active so peers don't renegotiate.
+    # Pin setup:active so browsers don't renegotiate when Meta echoes actpass.
     sdp_answer = payload.dig(:session, :sdp)&.gsub('a=setup:actpass', 'a=setup:active')
     update_call!(call, 'in_progress',
                  started_at: Time.current,
@@ -60,20 +54,17 @@ class Whatsapp::IncomingCallService
     return unless call
 
     duration = payload[:duration]&.to_i
-    update_call!(call, answered?(call, duration) ? 'completed' : 'no_answer',
-                 duration_seconds: duration, end_reason: payload[:terminate_reason])
-    broadcast(call, 'voice_call.ended', status: call.status, duration_seconds: call.duration_seconds)
+    status = answered?(call, duration) ? 'completed' : 'no_answer'
+    meta = (call.meta || {}).merge('ended_at' => Time.zone.now.to_i)
+    update_call!(call, status, duration_seconds: duration, end_reason: payload[:terminate_reason], meta: meta)
+    broadcast(call, 'voice_call.ended', status: call.display_status, duration_seconds: call.duration_seconds)
   end
 
-  # `accepted_by_agent_id` only signals an answered call for INBOUND — outbound
-  # calls have the initiating agent set before the contact picks up.
+  # accepted_by_agent_id is the initiating agent on outbound calls, so it only signals "answered" for inbound.
   def answered?(call, duration)
     call.in_progress? || duration.to_i.positive? || (call.incoming? && call.accepted_by_agent_id.present?)
   end
 
-  # The trio that always moves together when a call's status changes:
-  # update the Call row, refresh the message bubble, and sync the conversation
-  # additional_attributes for the FE.
   def update_call!(call, status, **attrs)
     call.update!(status: status, **attrs)
     Voice::CallMessageBuilder.update_status!(call: call, status: status, agent: call.accepted_by_agent,
@@ -89,22 +80,24 @@ class Whatsapp::IncomingCallService
     )
   end
 
-  # Ring only the conversation's assignee when assigned, account-wide otherwise
-  # so any eligible agent can pick up.
+  # Ring the assignee if assigned; otherwise account-wide so any agent can pick up.
   def broadcast_incoming(call, sdp_offer)
     contact = call.contact
-    data = base_payload(call).merge(
-      direction: call.direction_label, inbox_id: call.inbox_id,
-      sdp_offer: sdp_offer, ice_servers: Call.default_ice_servers,
-      caller: { name: contact.name, phone: contact.phone_number, avatar: contact.avatar_url }
-    )
-    streams = call.conversation.assignee&.pubsub_token ? [call.conversation.assignee.pubsub_token] : ["account_#{inbox.account_id}"]
-    streams.each { |s| ActionCable.server.broadcast(s, { event: 'voice_call.incoming', data: data }) }
+    token = call.conversation.assignee&.pubsub_token
+    broadcast(call, 'voice_call.incoming',
+              streams: token ? [token] : account_streams,
+              direction: call.direction_label, inbox_id: call.inbox_id,
+              sdp_offer: sdp_offer, ice_servers: Call.default_ice_servers,
+              caller: { name: contact.name, phone: contact.phone_number, avatar: contact.avatar_url })
   end
 
-  def broadcast(call, event, **extra)
-    ActionCable.server.broadcast("account_#{inbox.account_id}",
-                                 { event: event, data: base_payload(call).merge(extra) })
+  def broadcast(call, event, streams: account_streams, **extra)
+    payload = { event: event, data: base_payload(call).merge(extra) }
+    streams.each { |s| ActionCable.server.broadcast(s, payload) }
+  end
+
+  def account_streams
+    ["account_#{inbox.account_id}"]
   end
 
   def base_payload(call)

spec/enterprise/services/whatsapp/incoming_call_service_spec.rb

@@ -87,8 +87,10 @@ describe Whatsapp::IncomingCallService do
       described_class.new(inbox: inbox, params: params).perform
 
       expect(call.reload).to have_attributes(status: 'completed', duration_seconds: 42, end_reason: 'completed_normally')
+      expect(call.ended_at).to be_present
       expect(ActionCable.server).to have_received(:broadcast).with(
-        "account_#{account.id}", hash_including(event: 'voice_call.ended')
+        "account_#{account.id}",
+        hash_including(event: 'voice_call.ended', data: hash_including(status: 'completed'))
       )
     end
 
@@ -155,6 +157,10 @@ describe Whatsapp::IncomingCallService do
       described_class.new(inbox: inbox, params: params).perform
 
       expect(call.reload.status).to eq('no_answer')
+      expect(ActionCable.server).to have_received(:broadcast).with(
+        "account_#{account.id}",
+        hash_including(event: 'voice_call.ended', data: hash_including(status: 'no-answer'))
+      )
     end
   end