backend: clean up Twilio caller IDs when an account is deleted

backend/routers/users.py

@@ -91,6 +91,7 @@
 from database import user_usage as user_usage_db
 from utils import stripe as stripe_utils
 from utils.log_sanitizer import sanitize
+from utils.twilio_service import delete_user_caller_ids
 from utils.llm.followup import followup_question_prompt
 from utils.notifications import send_notification, send_training_data_submitted_notification
 from utils.llm.external_integrations import generate_comprehensive_daily_summary
@@ -142,6 +143,11 @@ class DeleteAccountRequest(BaseModel):
 
 def _background_wipe_user_data(uid: str):
     try:
+        # Twilio caller IDs first, while the phone_numbers subcollection still
+        # carries the twilio_sid metadata. delete_user_caller_ids is best-effort
+        # — Twilio errors are logged inside and never propagate, so a momentary
+        # Twilio outage cannot leave the user half-deleted in Firestore.
+        delete_user_caller_ids(uid)
         delete_user_data(uid)
         logger.info(f'delete_account background wipe complete for {uid}')
     except Exception as e:

backend/test.sh

@@ -126,6 +126,7 @@ pytest tests/unit/test_vertex_ai_system_role.py -v
 pytest tests/unit/test_tts.py -v
 pytest tests/unit/test_webhook_auto_disable.py -v
 pytest tests/unit/test_merge_validation.py -v
+pytest tests/unit/test_twilio_account_deletion.py -v
 
 # Fair-use integration tests (require Redis; skip gracefully if unavailable)
 if redis-cli ping >/dev/null 2>&1; then

backend/tests/unit/test_twilio_account_deletion.py

@@ -0,0 +1,111 @@
+import os
+import sys
+import types
+from unittest.mock import patch
+
+os.environ.setdefault('TWILIO_ACCOUNT_SID', 'ACtest123')
+os.environ.setdefault('TWILIO_AUTH_TOKEN', 'test_auth_token')
+os.environ.setdefault('TWILIO_API_KEY_SID', 'SKtest123')
+os.environ.setdefault('TWILIO_API_KEY_SECRET', 'test_api_secret')
+os.environ.setdefault('TWILIO_TWIML_APP_SID', 'APtest123')
+
+
+# Stub `database.phone_calls` before twilio_service tries to import it so we can
+# unit-test delete_user_caller_ids without dragging in firebase_admin and the
+# rest of the database init chain. The real implementation only uses
+# `get_phone_numbers`, which the tests override per-case via patch.object.
+def _install_phone_calls_stub():
+    if 'database' not in sys.modules:
+        sys.modules['database'] = types.ModuleType('database')
+    if 'database.phone_calls' not in sys.modules:
+        stub = types.ModuleType('database.phone_calls')
+
+        def get_phone_numbers(uid):
+            return []
+
+        stub.get_phone_numbers = get_phone_numbers
+        sys.modules['database.phone_calls'] = stub
+        setattr(sys.modules['database'], 'phone_calls', stub)
+
+
+_install_phone_calls_stub()
+
+from utils import twilio_service
+from database import phone_calls as phone_calls_db  # noqa: E402  (stub above)
+
+
+def test_delete_user_caller_ids_calls_twilio_for_each_sid():
+    numbers = [
+        {'id': 'a', 'twilio_sid': 'PNaaaa'},
+        {'id': 'b', 'twilio_sid': 'PNbbbb'},
+    ]
+    with patch.object(twilio_service, 'delete_caller_id', return_value=True) as mock_delete, patch.object(
+        phone_calls_db, 'get_phone_numbers', return_value=numbers
+    ):
+        deleted = twilio_service.delete_user_caller_ids('uid-1')
+    assert deleted == 2
+    assert mock_delete.call_count == 2
+    assert mock_delete.call_args_list[0].args[0] == 'PNaaaa'
+    assert mock_delete.call_args_list[1].args[0] == 'PNbbbb'
+
+
+def test_delete_user_caller_ids_skips_entries_without_sid():
+    numbers = [
+        {'id': 'a', 'twilio_sid': 'PNaaaa'},
+        {'id': 'b'},
+        {'id': 'c', 'twilio_sid': None},
+        {'id': 'd', 'twilio_sid': ''},
+    ]
+    with patch.object(twilio_service, 'delete_caller_id', return_value=True) as mock_delete, patch.object(
+        phone_calls_db, 'get_phone_numbers', return_value=numbers
+    ):
+        deleted = twilio_service.delete_user_caller_ids('uid-1')
+    assert deleted == 1
+    assert mock_delete.call_count == 1
+    assert mock_delete.call_args_list[0].args[0] == 'PNaaaa'
+
+
+def test_delete_user_caller_ids_continues_when_one_raises():
+    # Simulates _get_client() raising mid-loop (e.g. credentials were rotated
+    # and the cached client got reset between entries). Twilio API errors
+    # themselves don't reach this path — delete_caller_id swallows those and
+    # returns False — but the outer guard exists for _get_client() failures,
+    # and it must not abort cleanup of the remaining caller IDs.
+    numbers = [
+        {'id': 'a', 'twilio_sid': 'PNaaaa'},
+        {'id': 'b', 'twilio_sid': 'PNbbbb'},
+        {'id': 'c', 'twilio_sid': 'PNcccc'},
+    ]
+
+    def fake_delete(sid):
+        if sid == 'PNbbbb':
+            raise RuntimeError('twilio client unavailable')
+        return True
+
+    with patch.object(twilio_service, 'delete_caller_id', side_effect=fake_delete) as mock_delete, patch.object(
+        phone_calls_db, 'get_phone_numbers', return_value=numbers
+    ):
+        deleted = twilio_service.delete_user_caller_ids('uid-1')
+    assert deleted == 2
+    assert mock_delete.call_count == 3
+
+
+def test_delete_user_caller_ids_returns_zero_when_no_phone_numbers():
+    with patch.object(twilio_service, 'delete_caller_id', return_value=True) as mock_delete, patch.object(
+        phone_calls_db, 'get_phone_numbers', return_value=[]
+    ):
+        deleted = twilio_service.delete_user_caller_ids('uid-1')
+    assert deleted == 0
+    assert mock_delete.call_count == 0
+
+
+def test_delete_user_caller_ids_swallows_phone_list_error():
+    # If we can't even list the user's phone_numbers, we still must not raise —
+    # the caller (delete_account background wipe) needs to keep going so the
+    # Firestore wipe completes.
+    with patch.object(twilio_service, 'delete_caller_id', return_value=True) as mock_delete, patch.object(
+        phone_calls_db, 'get_phone_numbers', side_effect=RuntimeError('firestore down')
+    ):
+        deleted = twilio_service.delete_user_caller_ids('uid-1')
+    assert deleted == 0
+    assert mock_delete.call_count == 0

backend/utils/twilio_service.py

@@ -1,3 +1,4 @@
+import logging
 import os
 from typing import Optional
 
@@ -6,6 +7,10 @@
 from twilio.jwt.access_token.grants import VoiceGrant
 from twilio.request_validator import RequestValidator
 
+from database import phone_calls as phone_calls_db
+
+logger = logging.getLogger(__name__)
+
 account_sid = os.getenv('TWILIO_ACCOUNT_SID')
 auth_token = os.getenv('TWILIO_AUTH_TOKEN')
 api_key_sid = os.getenv('TWILIO_API_KEY_SID')
@@ -140,6 +145,39 @@ def delete_caller_id(sid: str) -> bool:
         return False
 
 
+def delete_user_caller_ids(uid: str) -> int:
+    # Delete every verified Twilio caller ID owned by `uid`.
+    #
+    # Returns the number of caller IDs successfully deleted. Best-effort by
+    # design — Twilio API errors are absorbed by delete_caller_id() itself
+    # (returns False), and the only way an exception reaches the outer guard
+    # is if _get_client() raises (e.g. missing TWILIO_* env vars). Both paths
+    # are logged but never propagate, so callers (account-deletion background
+    # wipe) can safely run this before a Firestore wipe without risking a
+    # half-deleted user if Twilio is misconfigured or momentarily down.
+    try:
+        numbers = phone_calls_db.get_phone_numbers(uid)
+    except Exception as e:
+        logger.error(f'delete_user_caller_ids: list phone_numbers failed: {e}')
+        return 0
+
+    deleted = 0
+    for number in numbers:
+        sid = number.get('twilio_sid')
+        if not sid:
+            continue
+        try:
+            if delete_caller_id(sid):
+                deleted += 1
+            else:
+                logger.warning(f'delete_user_caller_ids: twilio reported failure for sid={sid}')
+        except Exception as e:
+            # Reachable only if _get_client() itself raises before delete_caller_id's
+            # internal try/except — Twilio API errors are already swallowed there.
+            logger.error(f'delete_user_caller_ids: twilio client unavailable for sid={sid}: {e}')
+    return deleted
+
+
 def list_caller_ids() -> list:
     """
     List all verified outgoing caller IDs for the account.