Skip to content

fix(vapi): drop serverMessages — phone-number-level subscription#30

Merged
ByteStreams-AI merged 1 commit intomainfrom
fix/m11-drop-server-messages
May 5, 2026
Merged

fix(vapi): drop serverMessages — phone-number-level subscription#30
ByteStreams-AI merged 1 commit intomainfrom
fix/m11-drop-server-messages

Conversation

@ByteStreams-AI
Copy link
Copy Markdown
Owner

@ByteStreams-AI ByteStreams-AI commented May 5, 2026

Summary

PR #29's minimal serverMessages: ['end-of-call-report'] still triggered Vapi's "Couldn't get assistant" rejection. Field is removed entirely.

The end-of-call-report subscription has to live in the Vapi dashboard at the phone-number level, not on the dynamic assistant response:

Phone Numbers → +16296001047 → Server → Server Messages → add end-of-call-report (and any others you want like status-update).

Once that's configured in the dashboard, our existing dispatcher in vapi_call_start already handles the end-of-call-report envelope shape correctly, vapi_call_end fires, and the SMS dispatch path PR #27 wired comes alive.

Test plan

  • pnpm ci:fast — 299/299
  • After merge + deploy: place a test call → confirm "Couldn't get assistant" is gone
  • Configure phone-number-level Server Messages in Vapi dashboard (one-time, out-of-band of code)
  • After dashboard config: confirm voice_calls.ended_at is populated post-call
  • Confirm payment-link SMS arrives

Note for future contributors

Integration test asserts assistant.serverMessages === undefined. Any re-add of the field will fail the test and surface the May 5 2026 incident comment in vapi_call_start/index.ts.

🤖 Generated with Claude Code

Greptile Summary

Removes serverMessages: ['end-of-call-report'] from the dynamic assistant response in vapi_call_start after Vapi's validator rejected the field on two successive deploys (#28, #29). The subscription is moved to the phone-number-level Vapi dashboard config, and the test is updated to assert serverMessages stays undefined going forward. The code change itself is correct and minimal; the one thing worth noting is that the post-call pipeline now has an invisible out-of-band dependency on a dashboard setting that no automated check currently verifies.

Confidence Score: 4/5

Safe to merge; the code change is correct and the test guard is sound, with a minor operational risk from the unverifiable dashboard dependency.

No logic errors or security issues introduced. The only concern is a P2: the end-of-call pipeline now silently depends on a manual Vapi dashboard config with no programmatic verification, but this is an architectural trade-off explicitly acknowledged in the PR description rather than a defect in the changed code.

No files require special attention.

Important Files Changed

Filename Overview
supabase/functions/vapi_call_start/index.ts Removes serverMessages: ['end-of-call-report'] from dynamic assistant response; replaces with a detailed comment explaining the phone-number-level dashboard workaround.
packages/shared/test/db/voice.test.ts Updates assertion from .toContain('end-of-call-report') to .toBeUndefined() to guard against future re-addition of serverMessages.

Reviews (1): Last reviewed commit: "fix(vapi): drop serverMessages — phone-n..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Even the minimal `serverMessages: ['end-of-call-report']` from PR #29
caused Vapi to reject the assistant config with "Couldn't get
assistant". Removing the field entirely.

The end-of-call-report subscription needs to live at the phone-number
level in the Vapi dashboard:
  Phone Numbers → +16296001047 → Server → Server Messages →
    add 'end-of-call-report' (and 'tool-calls', 'status-update' if
    needed)

That config is out-of-band of this code and stays fixed across
calls. Once configured in the dashboard, the existing dispatcher in
vapi_call_start handles `end-of-call-report` envelope correctly and
the SMS dispatch path comes alive.

Code comment in vapi_call_start documents the phone-number-level
location so a future contributor doesn't try the field-level path
again. Integration test asserts serverMessages is UNDEFINED on the
dynamic assistant response — any re-add will fail this test and
surface the prior incident note.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment on lines +450 to +458
// NOTE: end-of-call-report subscription lives on the
// phone-number-level config in the Vapi dashboard (under
// Phone Numbers → +16296001047 → Server → Server Messages),
// NOT on this dynamic assistant response. Adding
// `serverMessages: [...]` here — even with a single value —
// caused Vapi's validator to reject the entire assistant
// config with "Couldn't get assistant" on the May 5 2026
// deploys (PRs #28 and #29). The phone-number-level path is
// out-of-band of this code and stays fixed across calls.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Operational dependency not verifiable by code

The post-call pipeline (vapi_call_end, voice_calls.ended_at, payment-link SMS) now depends entirely on a manual, one-time Vapi dashboard configuration. If that setting is absent — new environment, dashboard reset, or a second phone number is ever onboarded — events will silently not arrive and nothing in the codebase will surface the misconfiguration. Consider adding a startup or smoke-test assertion (e.g., a Vapi Management API call to verify the phone-number's serverMessages contains end-of-call-report) so the dependency is detectable without placing a live call.

@ByteStreams-AI ByteStreams-AI merged commit 975e1ae into main May 5, 2026
2 checks passed
@ByteStreams-AI ByteStreams-AI deleted the fix/m11-drop-server-messages branch May 5, 2026 02:32
ByteStreams-AI added a commit that referenced this pull request May 5, 2026
Two new docs to anchor the May 5, 2026 governance reset:

- docs/project-status2.md: operational + governance snapshot.
  Captures why we're resetting (PR #28#30 regression chain),
  what's working in cloud, what's broken (#32 voice path), the
  new governance rules, and the step-by-step path to stable
  production via #33 (staging) → #31 (prod branch) → #34
  (umbrella checklist).

- docs/git-policy.md: authoritative branching, commit, PR,
  merge, hotfix, migration, and rollback policy. Codifies the
  four governance rules: prod branch as deploy gate, user-only
  staging/commits, Issues-before-PRs, and every PR must
  reference an Issue.

docs/project-status.md (milestone tracker) and AGENTS.md (repo
conventions and follow-ups) stay as-is; these are additive.
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.

1 participant