feat(telephony): add WhatsApp calling integration with outbound Calling (Business-Initiated Calling & Permission Management) - #757
Open
amitbhakt wants to merge 9 commits into
Conversation
…nd permission management Integrates WhatsApp Business Calling API with WebRTC and Pipecat pipeline, supporting both User-Initiated (inbound) and Business-Initiated (outbound) calls with permission management and campaign dispatch integration. ## Backend - Provider: WhatsApp Business API implementation using WebRTC media transport - Inbound and outbound voice calling support - Geographic restriction validation (Meta policy exclusions for +1, +20, +84, +234) - Interactive call permission request messaging with 24-hour expiration - Dedicated service layer (service.py) decoupling active WebRTC connections, client caching, and Redis pub/sub from HTTP routes - Scoped recipient matching across phone format variations and telephony configurations - Campaign Orchestration & Dispatch: - Polymorphic TelephonyPermissionRequiredError handling in generic campaign dispatcher - Campaign orchestrator completion checks updated to retain parked runs - ARQ periodic cron task (sweep_parked_whatsapp_permissions) for automatic lead reactivation upon permission grant - Database & Migrations: - whatsapp_call_permissions model with unique constraint and indexes - Alembic migrations for permission tracking and parked queued runs index - Concurrency race handling with rollback and update retry on permission upsert - Webhooks & Security: - Meta webhook signature verification enforced across all endpoints including empty payloads - Webhook delivery and status processing for interactive permission responses ## Frontend - PhoneCallDialog: - Real-time WhatsApp call permission checking via backend API - Integrated permission request flow with custom message preview - Live call status progression, duration timer, and in-dialog termination controls - Campaigns: - WhatsAppPermissionCard component for managing campaign permission behavior (request_and_wait vs skip) - Campaign advanced settings and runs table integration ## Submodule & Tests - Updated Pipecat submodule with outbound calling and permission request methods - Comprehensive unit test suites covering restrictions, recipient matching, permissions gating, orchestrator lifecycle, concurrency races, and provider decoupling
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
6 issues found across 79 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="api/db/models.py">
<violation number="1" location="api/db/models.py:438">
P2: Every permission-message status webhook looks up `meta_message_id`, but this column has no index, so each callback scans the entire permissions table as it grows. Add a partial index on `meta_message_id` and include it in the migration.</violation>
</file>
<file name="api/db/workflow_run_client.py">
<violation number="1" location="api/db/workflow_run_client.py:120">
P2: This per-lead lookup has no index on `queued_run_id`, so campaign dispatches will repeatedly scan the full `workflow_runs` table at scale. Add a migration for an index on `queued_run_id` (ideally including `created_at` for this latest-row query).</violation>
</file>
<file name="docker-compose.override.yaml">
<violation number="1" location="docker-compose.override.yaml:11">
P2: This file is auto-loaded by every `docker compose` command in the repo, so it silently replaces the registry-backed stack with local builds and `pull_policy: never` for all deployments that clone the repo (start_docker.sh, remote_up.sh, update_remote.sh), not just opted-in local dev. Docstrings and the base compose drive deployment through registry pulls (`--pull always`); on a fresh host that has never built dograh-local images, `pull_policy: never` means nothing is pulled and the api/ui containers fail to start. The same content is already generated at runtime by setup_remote.sh only for build mode, so this should be opt-in rather than committed as a repo-root auto-loaded override.</violation>
</file>
<file name="api/db/telephony_configuration_client.py">
<violation number="1" location="api/db/telephony_configuration_client.py:30">
P2: When a permission row was stored with punctuation or spacing, a later canonical lookup misses it and inserts a second row for the same recipient. Compare canonicalized stored values or migrate/merge legacy rows before relying on this candidate list.</violation>
</file>
<file name="api/alembic/versions/b7d2f04c8a15_index_parked_queued_runs.py">
<violation number="1" location="api/alembic/versions/b7d2f04c8a15_index_parked_queued_runs.py:29">
P1: This index targets queued_runs.retry_reason, but that column is introduced by fefdd1835b7d (separate branch, down_revision a75ae71af479), which is not an ancestor of this migration. Applying this lineage on a clean database raises 'column queued_runs.retry_reason does not exist'. Set down_revision to a migration whose lineage includes the retry_reason column (merge the retry/inbound PR into this chain first), or this migration fails on fresh deploys.</violation>
</file>
<file name="api/services/telephony/providers/whatsapp/provider.py">
<violation number="1" location="api/services/telephony/providers/whatsapp/provider.py:300">
P1: When Meta successfully reports that permission is revoked or absent but the local status update fails, this fallback can dial using a stale local grant. Only use the local-permission fallback when `check_call_permission` itself is unavailable, not after a successful Meta response.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "Please generate a fresh token in Meta Business Manager and update your Telephony Configuration." | ||
| ), | ||
| ) | ||
| has_permission = _local_permission_is_usable(perm, now) |
Contributor
There was a problem hiding this comment.
P1: When Meta successfully reports that permission is revoked or absent but the local status update fails, this fallback can dial using a stale local grant. Only use the local-permission fallback when check_call_permission itself is unavailable, not after a successful Meta response.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/services/telephony/providers/whatsapp/provider.py, line 300:
<comment>When Meta successfully reports that permission is revoked or absent but the local status update fails, this fallback can dial using a stale local grant. Only use the local-permission fallback when `check_call_permission` itself is unavailable, not after a successful Meta response.</comment>
<file context>
@@ -0,0 +1,1231 @@
+ "Please generate a fresh token in Meta Business Manager and update your Telephony Configuration."
+ ),
+ )
+ has_permission = _local_permission_is_usable(perm, now)
+ else:
+ meta_checked = True
</file context>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…and permission lifecycle
Contributor
There was a problem hiding this comment.
4 issues found across 39 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="api/services/pipecat/event_handlers.py">
<violation number="1" location="api/services/pipecat/event_handlers.py:87">
P2: Existing `asyncio.Event` callers now fail because `Event.wait()` returns `True`, not `ANSWERED`, so the handler skips recording and the greeting. Update those callers and tests to use `OutboundCallGate.resolve(ANSWERED)`, or explicitly preserve compatibility with `asyncio.Event`.</violation>
</file>
<file name="api/db/campaign_client.py">
<violation number="1" location="api/db/campaign_client.py:27">
P2: Tightening is_same_recipient_number to require full canonical-digit equality (plus the matching exact-digit SQL prefilter in get_queued_runs_awaiting_whatsapp_permission) means any parked lead stored without its country code will never be reactivated by a permission grant webhook or cron sweep, so the run strands until the 24h park expiry and fails even though the recipient consented. The new docstring assumes every stored lead starts with '+' (validated at ingest), but parked runs created from legacy or pre-validation data are not guaranteed to satisfy that. If any such rows exist, the grant path silently stops working for them. Confirm no legacy leads lack the '+' prefix, or run an ingest-normalization migration first.</violation>
</file>
<file name="api/db/models.py">
<violation number="1" location="api/db/models.py:468">
P2: `update_whatsapp_call_permission_status_by_message_id` resolves the row with `scalars().first()` and no ORDER BY, but the new `ix_whatsapp_perm_meta_message_id` is deliberately non-unique. If a wamid ever spans two rows (the exact case the models comment says the webhook must 'degrade to an extra row' for), the update lands on an arbitrary row, granting/denying the wrong recipient and moving the wrong campaign run. Order the lookup deterministically (e.g. by id) so a replayed wamid always updates the same row.</violation>
<violation number="2" location="api/db/models.py:470">
P3: Permission updates never clear `meta_message_id` after a request finishes, so this index accumulates historical rows instead of the claimed outstanding-request slice. Remove the partial predicate or clear IDs through a defined terminal-state lifecycle.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| Index( | ||
| "ix_whatsapp_perm_meta_message_id", | ||
| "meta_message_id", | ||
| postgresql_where=text("meta_message_id IS NOT NULL"), |
Contributor
There was a problem hiding this comment.
P3: Permission updates never clear meta_message_id after a request finishes, so this index accumulates historical rows instead of the claimed outstanding-request slice. Remove the partial predicate or clear IDs through a defined terminal-state lifecycle.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/db/models.py, line 470:
<comment>Permission updates never clear `meta_message_id` after a request finishes, so this index accumulates historical rows instead of the claimed outstanding-request slice. Remove the partial predicate or clear IDs through a defined terminal-state lifecycle.</comment>
<file context>
@@ -458,6 +458,17 @@ class WhatsAppCallPermissionModel(Base):
+ Index(
+ "ix_whatsapp_perm_meta_message_id",
+ "meta_message_id",
+ postgresql_where=text("meta_message_id IS NOT NULL"),
+ ),
)
</file context>
…tcher mocks, cooldown timing, and E.164 validation - Use atomic Redis counter in Lua script to prevent token collision in rate_limiter - Restore acquire_from_number on CampaignCallDispatcher and configure token mocks in tests - Arm WhatsApp permission sync cooldown only after validating parked runs and config - Enforce strict E.164 validation and isolate route checks to WhatsApp provider
Contributor
There was a problem hiding this comment.
3 issues found across 30 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="api/services/campaign/campaign_call_dispatcher.py">
<violation number="1" location="api/services/campaign/campaign_call_dispatcher.py:307">
P3: This PR removed the last production caller of `db_client.increment_campaign_processed_rows`, leaving the method (and its SQL in campaign_client.py:636-661) dead code. Delete it to avoid confusion with the new recompute-based `sync_campaign_processed_rows`.</violation>
</file>
<file name="api/db/telephony_phone_number_client.py">
<violation number="1" location="api/db/telephony_phone_number_client.py:414">
P2: When concurrent requests race to set different default caller IDs, this branch raises `TelephonyPhoneNumberConflictError`, but the endpoint leaves it unhandled and returns a generic 500. Catch this exception in the route and return the same 409 conflict response used for phone-number conflicts.</violation>
</file>
<file name="api/routes/campaign.py">
<violation number="1" location="api/routes/campaign.py:580">
P2: When a user clicks sync during the 30-second cooldown, this response sets `throttled` but the campaign page ignores it and falsely reports that no recipient granted permission. Update the client type and UI to distinguish a cooldown retry from a completed sync.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| "reactivated_count": result.reactivated, | ||
| # Distinguishes "skipped, try again shortly" from "asked Meta, nobody | ||
| # has granted permission yet" - both reactivate zero runs. | ||
| "throttled": result.throttled, |
Contributor
There was a problem hiding this comment.
P2: When a user clicks sync during the 30-second cooldown, this response sets throttled but the campaign page ignores it and falsely reports that no recipient granted permission. Update the client type and UI to distinguish a cooldown retry from a completed sync.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/routes/campaign.py, line 580:
<comment>When a user clicks sync during the 30-second cooldown, this response sets `throttled` but the campaign page ignores it and falsely reports that no recipient granted permission. Update the client type and UI to distinguish a cooldown retry from a completed sync.</comment>
<file context>
@@ -562,17 +562,22 @@ async def sync_campaign_whatsapp_permissions(
+ "reactivated_count": result.reactivated,
+ # Distinguishes "skipped, try again shortly" from "asked Meta, nobody
+ # has granted permission yet" - both reactivate zero runs.
+ "throttled": result.throttled,
}
</file context>
| # commits then race for uq_phone_numbers_default_caller. Same | ||
| # failure mode create_phone_number already handles. | ||
| await session.rollback() | ||
| raise TelephonyPhoneNumberConflictError(str(e)) from e |
Contributor
There was a problem hiding this comment.
P2: When concurrent requests race to set different default caller IDs, this branch raises TelephonyPhoneNumberConflictError, but the endpoint leaves it unhandled and returns a generic 500. Catch this exception in the route and return the same 409 conflict response used for phone-number conflicts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/db/telephony_phone_number_client.py, line 414:
<comment>When concurrent requests race to set different default caller IDs, this branch raises `TelephonyPhoneNumberConflictError`, but the endpoint leaves it unhandled and returns a generic 500. Catch this exception in the route and return the same 409 conflict response used for phone-number conflicts.</comment>
<file context>
@@ -402,7 +402,16 @@ async def update_phone_number(
+ # commits then race for uq_phone_numbers_default_caller. Same
+ # failure mode create_phone_number already handles.
+ await session.rollback()
+ raise TelephonyPhoneNumberConflictError(str(e)) from e
await session.refresh(row)
return row
</file context>
| return | ||
|
|
||
| try: | ||
| actual = await db_client.sync_campaign_processed_rows(campaign_id) |
Contributor
There was a problem hiding this comment.
P3: This PR removed the last production caller of db_client.increment_campaign_processed_rows, leaving the method (and its SQL in campaign_client.py:636-661) dead code. Delete it to avoid confusion with the new recompute-based sync_campaign_processed_rows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/services/campaign/campaign_call_dispatcher.py, line 307:
<comment>This PR removed the last production caller of `db_client.increment_campaign_processed_rows`, leaving the method (and its SQL in campaign_client.py:636-661) dead code. Delete it to avoid confusion with the new recompute-based `sync_campaign_processed_rows`.</comment>
<file context>
@@ -275,50 +285,34 @@ async def process_batch(self, campaign_id: int, batch_size: int = 10) -> int:
try:
- await db_client.increment_campaign_processed_rows(
- campaign_id=campaign_id, delta=delta
+ actual = await db_client.sync_campaign_processed_rows(campaign_id)
+ logger.debug(
+ f"Campaign {campaign_id} processed_rows synced to {actual}"
</file context>
…ion sync and layering - Tie the pipeline teardown claim to the task's lifetime so an abandoned pipeline cannot re-terminate a call after the canceller stops waiting - Extract campaign permission orchestration into permission_sync.py, keeping service.py to call transport and lifecycle - Make sync_campaign_processed_rows the only writer of processed_rows, locking the campaign row before counting - Honour the activation/denial claim result so duplicate grants no longer over-report or enqueue extra campaign batches - Move generate_turn_credentials into api/services/turn.py and fix the TTL argument that passed TURN_SECRET as an int - Centralise WhatsApp permission status classification in config.py - Give the campaign sync endpoint a response model and use the generated SDK operation in the UI - Refuse an ambiguous webhook verify-token match across configurations - Repoint tests at the modules the code now lives in; regenerate client
Comment on lines
+72
to
+79
| if not is_e164(phone_number): | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail=( | ||
| "Phone number must be in strict E.164 format, including the country " | ||
| f"code with a leading '+' (e.g. +14155552671). Got: {phone_number!r}" | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Campaign ingestion accepts and retains formatted international numbers such as +44 7123 456789, but WhatsApp dispatch rejects the same stored value because it requires punctuation-free E.164. Those valid campaign contacts are marked failed before a call is placed. Normalize the value during ingestion or apply the same strict validation before storing it.
Artifacts
- A narrow Python reproduction invokes the production campaign validator and WhatsApp destination validator with the formatted UK number, showing the exact exercised path.
- Captured execution output shows ingestion accepted the formatted number and dispatch returned HTTP 400, confirming the incompatible validation rules.
Contributor
There was a problem hiding this comment.
1 issue found across 35 files (changes from recent commits).
Not reviewed (too large): ui/src/client/index.ts (~4 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="api/services/telephony/providers/whatsapp/restrictions.py">
<violation number="1" location="api/services/telephony/providers/whatsapp/restrictions.py:72">
P1: Formatted international campaign numbers pass ingestion but are rejected here before outbound calling. Canonicalize the destination before this check, or enforce the same strict `is_e164` rule during campaign ingestion.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| """ | ||
| from api.utils.telephony_address import is_e164 | ||
|
|
||
| if not is_e164(phone_number): |
Contributor
There was a problem hiding this comment.
P1: Formatted international campaign numbers pass ingestion but are rejected here before outbound calling. Canonicalize the destination before this check, or enforce the same strict is_e164 rule during campaign ingestion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/services/telephony/providers/whatsapp/restrictions.py, line 72:
<comment>Formatted international campaign numbers pass ingestion but are rejected here before outbound calling. Canonicalize the destination before this check, or enforce the same strict `is_e164` rule during campaign ingestion.</comment>
<file context>
@@ -83,11 +62,22 @@ def is_restricted_country(phone_number: str) -> Tuple[bool, Optional[str]]:
"""
+ from api.utils.telephony_address import is_e164
+
+ if not is_e164(phone_number):
+ raise HTTPException(
+ status_code=400,
</file context>
…rom provider capability - Normalize lead phone numbers to strict E.164 at campaign ingest and canonicalize again on the WhatsApp dial path, so a formatted number accepted at upload is no longer failed before a call is placed - Add ProviderSpec.requires_call_permission and surface it on the telephony configuration responses; the campaign UI branches on that capability instead of comparing provider names - Classify Meta permission status once from the normalized value so a permanent grant is not stored as temporary without an expiry - Recount processed_rows once per campaign after denials instead of once per queued run, which serialized exclusive locks on the webhook path - Defer the whole teardown to handle_call_terminate when it claimed the pipeline, rather than repeating pops and slot releases in the finalizer - Move generate_turn_credentials to api/services/turn.py and merge the split api.constants imports it left behind - Refuse an ambiguous webhook verify-token match across configurations - Cover the dial-path validation, the per-campaign recount and the import isolation rule with tests that fail when the behavior regresses
Contributor
There was a problem hiding this comment.
1 issue found across 35 files (changes from recent commits).
Not reviewed (too large): ui/src/client/index.ts (~4 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="api/services/campaign/sources/csv.py">
<violation number="1" location="api/services/campaign/sources/csv.py:41">
P2: When a CSV contains the same number in different accepted formatting variants, campaign validation accepts both rows, then this assignment canonicalizes them to the same stored number and the dispatcher places duplicate calls. Normalize phone numbers before duplicate validation or reject duplicates using their canonical E.164 values.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| # exactly as supplied, so validation still reports it the same way. | ||
| canonical_phone = canonicalize_e164(context_vars.get("phone_number")) | ||
| if canonical_phone: | ||
| context_vars["phone_number"] = canonical_phone |
Contributor
There was a problem hiding this comment.
P2: When a CSV contains the same number in different accepted formatting variants, campaign validation accepts both rows, then this assignment canonicalizes them to the same stored number and the dispatcher places duplicate calls. Normalize phone numbers before duplicate validation or reject duplicates using their canonical E.164 values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/services/campaign/sources/csv.py, line 41:
<comment>When a CSV contains the same number in different accepted formatting variants, campaign validation accepts both rows, then this assignment canonicalizes them to the same stored number and the dispatcher places duplicate calls. Normalize phone numbers before duplicate validation or reject duplicates using their canonical E.164 values.</comment>
<file context>
@@ -30,6 +31,15 @@ def _build_context_variables(
+ # exactly as supplied, so validation still reports it the same way.
+ canonical_phone = canonicalize_e164(context_vars.get("phone_number"))
+ if canonical_phone:
+ context_vars["phone_number"] = canonical_phone
+
override_type = (
</file context>
Comment on lines
+39
to
+41
| canonical_phone = canonicalize_e164(context_vars.get("phone_number")) | ||
| if canonical_phone: | ||
| context_vars["phone_number"] = canonical_phone |
There was a problem hiding this comment.
CSV validation compares the raw phone strings before this code canonicalizes them. A file containing +44 7123 456789 and +447123456789 is accepted, then creates two queued runs with the same canonical recipient, +447123456789. That recipient can receive duplicate permission requests or outbound calls. Canonicalize phone numbers before validating duplicates.
Knowledge Base Used:
Artifacts
- Shows the authored Python harness that executes validation and sync for HEAD^ and the checked-out PR, ending with the takeaway that the actual bulk-insert payload is inspected.
- Runs the harness against HEAD^; validation passes and two queued rows retain distinct raw phone values, establishing the pre-change baseline.
- Runs the harness against the checked-out revision; validation passes, two rows are queued, and both persisted context phones are the identical canonical value, establishing the regression.
…egistry-driven UI - Canonicalize lead numbers to strict E.164 at campaign ingest and again on the WhatsApp dial path, dropping a bracketed national trunk prefix so '+44 (0) 20 7946 0958' does not become a valid-looking wrong number - Detect duplicate leads by canonical number, so two spellings of one contact no longer produce two queued runs - Shield the terminate owner's cleanup: cancelling it mid-disconnect no longer leaves a run incomplete with its concurrency slot held - Recount processed_rows once per campaign, from a finally so partial denials still update progress - Drive the campaign UI from ProviderSpec.requires_call_permission instead of provider names, read off the registry in the route rather than through the shared telephony factory - Keep the no-phone-numbers warning visible for permission-based providers - Replace the formatted-destination test that hit Meta's live API with a mocked one asserting the canonical number reaches every downstream step
Contributor
There was a problem hiding this comment.
2 issues found across 40 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="ui/src/app/campaigns/CampaignAdvancedSettings.tsx">
<violation number="1" location="ui/src/app/campaigns/CampaignAdvancedSettings.tsx:153">
P3: On the campaign edit page the newly ungated "No phone numbers configured" warning can never render for a consent-based (WhatsApp) provider: edit/page.tsx passes `fromNumbersCount={effectiveFromNumbers}` where `effectiveFromNumbers = requiresCallPermission ? 1 : ...`, so `fromNumbersCount === 0` is always false there. The comment claims the gate was removed so the warning also appears for consent-based providers, but only the new-campaign page (which passes the real `phone_number_count`) actually shows it. Pass the real number count to the warning (separate from the concurrency sentinel) so the edit page can surface it too.</violation>
</file>
<file name="api/db/campaign_client.py">
<violation number="1" location="api/db/campaign_client.py:1090">
P3: The new regexp_replace prefilter is not a strict superset of is_same_recipient_number, despite the comment claiming it matches 'exactly the reduction' the matcher performs. is_same_recipient_number also accepts via normalize_telephony_address(...).canonical == target_canonical, which strips a bracketed national trunk prefix: '+44 (0) 20 7946 0958' → '+442079460958'. SQL digit-stripping keeps that '0', so such a stored row fails the regexp_replace equality and is dropped before the Python matcher sees it. Ingest currently canonicalizes leads to E.164 before storage, which hides the gap, but the comment's own premise is that stored numbers can be formatted — for that case the filter still misses the trunk-prefix spelling. Tighten the SQL reduction to also drop a bracketed '(0)' (or at least correct the comment so it does not claim the prefilter is a superset of the matcher).</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| consent-based provider does not have, but every provider | ||
| still needs an active number to dial from, so hiding this | ||
| hid the one warning that the campaign cannot run at all. */} | ||
| {fromNumbersCount === 0 && ( |
Contributor
There was a problem hiding this comment.
P3: On the campaign edit page the newly ungated "No phone numbers configured" warning can never render for a consent-based (WhatsApp) provider: edit/page.tsx passes fromNumbersCount={effectiveFromNumbers} where effectiveFromNumbers = requiresCallPermission ? 1 : ..., so fromNumbersCount === 0 is always false there. The comment claims the gate was removed so the warning also appears for consent-based providers, but only the new-campaign page (which passes the real phone_number_count) actually shows it. Pass the real number count to the warning (separate from the concurrency sentinel) so the edit page can surface it too.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ui/src/app/campaigns/CampaignAdvancedSettings.tsx, line 153:
<comment>On the campaign edit page the newly ungated "No phone numbers configured" warning can never render for a consent-based (WhatsApp) provider: edit/page.tsx passes `fromNumbersCount={effectiveFromNumbers}` where `effectiveFromNumbers = requiresCallPermission ? 1 : ...`, so `fromNumbersCount === 0` is always false there. The comment claims the gate was removed so the warning also appears for consent-based providers, but only the new-campaign page (which passes the real `phone_number_count`) actually shows it. Pass the real number count to the warning (separate from the concurrency sentinel) so the edit page can surface it too.</comment>
<file context>
@@ -145,9 +145,14 @@ export default function CampaignAdvancedSettings({
+ consent-based provider does not have, but every provider
+ still needs an active number to dial from, so hiding this
+ hid the one warning that the campaign cannot run at all. */}
+ {fromNumbersCount === 0 && (
<p className="text-sm text-amber-600 dark:text-amber-400">
- No phone numbers configured. Add CLIs in <Link href="/telephony-configurations" className="underline font-medium">Telephony Configuration</Link> before running the campaign.
</file context>
| candidate_filters = [ | ||
| QueuedRunModel.context_variables["phone_number"].as_string() == raw_trimmed, | ||
| QueuedRunModel.context_variables["phone_number"].as_string() | ||
| == target_digits, |
Contributor
There was a problem hiding this comment.
P3: The new regexp_replace prefilter is not a strict superset of is_same_recipient_number, despite the comment claiming it matches 'exactly the reduction' the matcher performs. is_same_recipient_number also accepts via normalize_telephony_address(...).canonical == target_canonical, which strips a bracketed national trunk prefix: '+44 (0) 20 7946 0958' → '+442079460958'. SQL digit-stripping keeps that '0', so such a stored row fails the regexp_replace equality and is dropped before the Python matcher sees it. Ingest currently canonicalizes leads to E.164 before storage, which hides the gap, but the comment's own premise is that stored numbers can be formatted — for that case the filter still misses the trunk-prefix spelling. Tighten the SQL reduction to also drop a bracketed '(0)' (or at least correct the comment so it does not claim the prefilter is a superset of the matcher).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/db/campaign_client.py, line 1090:
<comment>The new regexp_replace prefilter is not a strict superset of is_same_recipient_number, despite the comment claiming it matches 'exactly the reduction' the matcher performs. is_same_recipient_number also accepts via normalize_telephony_address(...).canonical == target_canonical, which strips a bracketed national trunk prefix: '+44 (0) 20 7946 0958' → '+442079460958'. SQL digit-stripping keeps that '0', so such a stored row fails the regexp_replace equality and is dropped before the Python matcher sees it. Ingest currently canonicalizes leads to E.164 before storage, which hides the gap, but the comment's own premise is that stored numbers can be formatted — for that case the filter still misses the trunk-prefix spelling. Tighten the SQL reduction to also drop a bracketed '(0)' (or at least correct the comment so it does not claim the prefilter is a superset of the matcher).</comment>
<file context>
@@ -1086,10 +1086,14 @@ async def get_queued_runs_awaiting_whatsapp_permission(
- QueuedRunModel.context_variables["phone_number"].as_string() == target_canonical,
- QueuedRunModel.context_variables["phone_number"].as_string() == target_no_plus,
+ QueuedRunModel.context_variables["phone_number"].as_string()
+ == target_digits,
+ QueuedRunModel.context_variables["phone_number"].as_string()
+ == f"+{target_digits}",
</file context>
…one-number count - Only drop a bracketed '(0)' directly after the country code: the previous pattern matched one anywhere in the number and deleted a subscriber digit, changing the call destination - Pass the configured phone-number count to CampaignAdvancedSettings separately from the concurrency sentinel, so the 'no phone numbers' warning can render on the edit page for consent-based providers - Regenerate docs/api-reference/openapi.json for the routes and schema fields added earlier in this branch
Contributor
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="ui/src/app/campaigns/new/page.tsx">
<violation number="1" location="ui/src/app/campaigns/new/page.tsx:533">
P3: configuredPhoneNumberCount is passed the exact same expression as fromNumbersCount (line 532 uses availableFromNumbersCount, which line 232 defines as selectedTelephonyConfig?.phone_number_count ?? fromNumbersCount). This page never substitutes a concurrency sentinel for fromNumbersCount the way the edit page does, so the new prop is a no-op and the component's (configuredPhoneNumberCount ?? fromNumbersCount) === 0 warning falls back to the identical value. Remove the redundant prop (the warning then correctly uses fromNumbersCount) or, if a real distinction is intended, pass the selected config's actual count while keeping the fallback distinct.</violation>
</file>
<file name="api/utils/telephony_address.py">
<violation number="1" location="api/utils/telephony_address.py:58">
P1: When formatting splits a three-digit country code, this pattern fails to recognize the following `(0)` trunk prefix and leaves `0` in the canonical number. Allow formatting between the country-code digits while still limiting the prefix to at most three digits.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| # directly after it is the trunk prefix. One appearing later is part of the | ||
| # subscriber number's punctuation, and dropping its digit would change who | ||
| # gets dialled - so that case falls through to plain bracket stripping. | ||
| _TRUNK_PREFIX_RE = re.compile(r"^(\+\s*[0-9]{1,3}[\s\-.]*)\(\s*0\s*\)") |
Contributor
There was a problem hiding this comment.
P1: When formatting splits a three-digit country code, this pattern fails to recognize the following (0) trunk prefix and leaves 0 in the canonical number. Allow formatting between the country-code digits while still limiting the prefix to at most three digits.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/utils/telephony_address.py, line 58:
<comment>When formatting splits a three-digit country code, this pattern fails to recognize the following `(0)` trunk prefix and leaves `0` in the canonical number. Allow formatting between the country-code digits while still limiting the prefix to at most three digits.</comment>
<file context>
@@ -50,7 +50,12 @@ class NormalizedAddress:
+# directly after it is the trunk prefix. One appearing later is part of the
+# subscriber number's punctuation, and dropping its digit would change who
+# gets dialled - so that case falls through to plain bracket stripping.
+_TRUNK_PREFIX_RE = re.compile(r"^(\+\s*[0-9]{1,3}[\s\-.]*)\(\s*0\s*\)")
</file context>
Suggested change
| _TRUNK_PREFIX_RE = re.compile(r"^(\+\s*[0-9]{1,3}[\s\-.]*)\(\s*0\s*\)") | |
| _TRUNK_PREFIX_RE = re.compile(r"^(\+\s*[0-9](?:[\s\-.]*[0-9]){0,2}[\s\-.]*)\(\s*0\s*\)") |
| orgConcurrentLimit={orgConcurrentLimit} | ||
| fromNumbersCount={fromNumbersCount} | ||
| fromNumbersCount={availableFromNumbersCount} | ||
| configuredPhoneNumberCount={selectedTelephonyConfig?.phone_number_count ?? fromNumbersCount} |
Contributor
There was a problem hiding this comment.
P3: configuredPhoneNumberCount is passed the exact same expression as fromNumbersCount (line 532 uses availableFromNumbersCount, which line 232 defines as selectedTelephonyConfig?.phone_number_count ?? fromNumbersCount). This page never substitutes a concurrency sentinel for fromNumbersCount the way the edit page does, so the new prop is a no-op and the component's (configuredPhoneNumberCount ?? fromNumbersCount) === 0 warning falls back to the identical value. Remove the redundant prop (the warning then correctly uses fromNumbersCount) or, if a real distinction is intended, pass the selected config's actual count while keeping the fallback distinct.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ui/src/app/campaigns/new/page.tsx, line 533:
<comment>configuredPhoneNumberCount is passed the exact same expression as fromNumbersCount (line 532 uses availableFromNumbersCount, which line 232 defines as selectedTelephonyConfig?.phone_number_count ?? fromNumbersCount). This page never substitutes a concurrency sentinel for fromNumbersCount the way the edit page does, so the new prop is a no-op and the component's (configuredPhoneNumberCount ?? fromNumbersCount) === 0 warning falls back to the identical value. Remove the redundant prop (the warning then correctly uses fromNumbersCount) or, if a real distinction is intended, pass the selected config's actual count while keeping the fallback distinct.</comment>
<file context>
@@ -530,6 +530,7 @@ export default function NewCampaignPage() {
effectiveLimit={effectiveLimit}
orgConcurrentLimit={orgConcurrentLimit}
fromNumbersCount={availableFromNumbersCount}
+ configuredPhoneNumberCount={selectedTelephonyConfig?.phone_number_count ?? fromNumbersCount}
retryEnabled={retryEnabled}
onRetryEnabledChange={setRetryEnabled}
</file context>
…d-calling # Conflicts: # api/services/campaign/source_sync.py # api/services/pipecat/event_handlers.py # pipecat # ui/src/client/index.ts # ui/src/client/sdk.gen.ts # ui/src/client/types.gen.ts
… count, type WhatsApp dialog anys
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request: WhatsApp Outbound Calling (Business-Initiated Calling & Permission Management)
1. Summary
While the prerequisite Inbound PR introduced the base WhatsApp telephony provider for user-initiated incoming calls, this pull request implements Business-Initiated Calling (BIC).
Because Meta platform regulations require explicit recipient consent prior to initiating outbound WhatsApp calls, this PR introduces:
service.py): Establishes a dedicated service layer for active WebRTC connections, client caching, and Redis pub/sub, removing all provider dependencies on HTTP route handlers.2. Architecture and Technical Design
2.1 Provider Decoupling via Dedicated Service Layer (
service.py)In the initial implementation,
WhatsAppProviderimported internal connection globals and helpers directly fromroutes.py. This PR decouples the architecture:service.py: Encapsulates:_active_connections)._outbound_answered_events).whatsapp:call:terminate,whatsapp:call:events).WhatsAppClientinstances.set_pipeline_runner), enabling the provider to register active calls without directly importing route modules.provider.py: Imports exclusively fromservice.py. It initiates outbound calls via WebRTC after validating country eligibility and checking local recipient permission records.2.2 Destination Country Policy Restrictions (
restrictions.py)Meta strictly restricts Business-Initiated WhatsApp Calls in specific jurisdictions:
+1)+20)+84)+234)restrictions.pyvalidates destination numbers before any API call is made. Phone numbers belonging to restricted jurisdictions fail fast withDestinationCountryRestrictedError. The frontend queries the backend/permissions/checkendpoint directly to display policy notices, ensuring a single source of truth without duplicated client-side prefix matching.2.3 Recipient Permission Lifecycle and Concurrency-Safe Upserts
Outbound calls require an active permission record (
granted_temporaryorgranted_permanent):+, and local variants to prevent duplicate rows or missed permissions across formatting differences.call_permission_requestpayloads when permission is absent or expired.upsert_whatsapp_call_permissionhandles PostgreSQL unique constraint collisions (uq_whatsapp_perm_config_recipient) caused by simultaneous webhook deliveries. On conflict, the transaction rolls back and safely updates the existing record.2.4 Campaign Dispatcher and Orchestrator Integration
TelephonyPermissionRequiredErrorinapi/services/telephony/base.py.WhatsAppPermissionRequiredErrorinherits from this class.campaign_call_dispatcher.pyhandles permission-gated calls polymorphically without importing provider-specific classes.whatsapp_permission_action = "request_and_wait", unpermitted leads receive a permission request and their run is parked withscheduled_for = now + 24hinqueuedstate.campaign_orchestrator.pychecks total queued and processing runs before marking a campaign complete. Campaigns containing parked leads are prevented from completing prematurely while awaiting recipient responses.sweep_parked_whatsapp_permissions): Registered an ARQ cron task running every 2 minutes inWorkerSettings.cron_jobsto poll for granted permissions and reactivate parked runs automatically.3. Database Schema Changes
This PR includes two Alembic migrations:
e1a2b3c4d5e6_add_whatsapp_call_permissions.py:whatsapp_call_permissionstable:id,organization_id,telephony_configuration_id,phone_number_id,recipient_phone_number,status,permission_type,meta_message_id,requested_at,granted_at,expires_at,updated_at.uq_whatsapp_perm_config_recipienton(telephony_configuration_id, recipient_phone_number).ix_whatsapp_perm_lookupon(phone_number_id, recipient_phone_number).b7d2f04c8a15_index_parked_queued_runs.py:ix_queued_runs_retry_reason_stateon(retry_reason, state)to optimize queries for parked leads during cron sweeps.4. Frontend Enhancements
4.1 Phone Call Dialog (
PhoneCallDialog.tsx)/permissions/check.granted,pending,not_requested,restricted).4.2 Campaign Settings (
WhatsAppPermissionCard.tsx)request_and_wait: Sends permission request and parks the lead for up to 24 hours.skip: Skips unpermitted leads immediately.5. Pipecat Submodule Updates
Updates the
pipecatsubmodule pointer to include:WhatsAppClient.initiate_outbound_call: Creates local SDP offer and sends it to Meta Graph API via action"connect".WhatsAppClient.check_call_permission: Queries Meta'scall_permissionsAPI.WhatsAppClient.send_call_permission_request: Dispatches interactive permission messages.SmallWebRTCClient.6. Testing and Verification Guide
6.1 Automated Test Execution
6.2 Key Test Scenarios Covered
+1,+20,+84, and+234raiseDestinationCountryRestrictedErrorand are rejected before placing any API call.WhatsAppPermissionRequiredError(andTelephonyPermissionRequiredError).upsert_whatsapp_call_permissionverify rollback onIntegrityErrorand successful retry update.sweep_parked_whatsapp_permissionsreactivates parked leads when permission is granted.WhatsAppProviderimports fromservice.pywith zero imports fromroutes.py.7. Reviewer Checklist
e1a2b3c4d5e6andb7d2f04c8a15underapi/alembic/versions/.provider.pydoes not reference route handlers and delegates state toservice.py.TelephonyPermissionRequiredErrorinapi/services/telephony/base.pyand its handling incampaign_call_dispatcher.py.sweep_parked_whatsapp_permissionsis registered inapi/tasks/arq.pyunderWorkerSettings.cron_jobs.PhoneCallDialog.tsxuses backend/permissions/checkresponse without hardcoded client-side country lists.Summary by cubic
Adds WhatsApp Business-Initiated Calling (outbound) with recipient permission management, campaign lead parking, and a new outbound call UI. Depends on the inbound calling PR (
feature/whatsapp-inbound-calling); merge that first.+1,+20,+84,+234); campaign numbers are canonicalized to E.164 at ingest so formatted uploads dial correctly, and a bracketed national trunk prefix is stripped only when it directly follows the country code.whatsapp_call_permissionstable, sends interactive permission requests when missing, and matches permissions by exact canonical digit equality.request_and_waitmode and reactivates them on consent via a 2-minute ARQ sweep; the sync cooldown only arms after parked runs and config are validated.processed_rowsonce per campaign instead of once per queued run.PhoneCallDialogUI with live call timer, hangup, and custom permission message; campaign UI branches on a providerrequires_call_permissioncapability flag and reads the phone-number count separately from the concurrency sentinel so the missing-numbers warning still renders.service.pylayer; the pipeline runner installs at FastAPI and ARQ startup instead of via import side effects.Migration
e1a2b3c4d5e6,b7d2f04c8a15, andc3f5a1b90d47.WHATSAPP_WEBHOOK_VERIFY_TOKENfor webhook verification; updates thepipecatsubmodule (rebuild needed).Written for commit 22632e8. Summary will update on new commits.
Safe to merge; no outstanding blocking issues remain.
Findings
Summary
Reviews (8) · Last reviewed commit: "fix(telephony): handle split country cod..."