fix: address review feedback for calls feature#5255
Conversation
…ate, provider termination, error handling Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcb5bf1e23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (session.providerCallSid) { | ||
| try { | ||
| const provider = new TwilioConversationRelayProvider(); | ||
| await provider.endCall(session.providerCallSid); |
There was a problem hiding this comment.
Avoid blocking teardown on provider hangup call
This await provider.endCall(...) runs before relay/orchestrator cleanup, so call termination now depends on Twilio responsiveness; when Twilio/network is slow or stalls, call_end can hang and never reach the local cleanup path, leaving the active relay/orchestrator alive instead of ending promptly. Because endCall uses a plain fetch without an abort timeout, this regression can surface during provider incidents and prevents the tool from reliably stopping calls.
Useful? React with 👍 / 👎.
| updateCallSession(sessionId, { | ||
| status: 'failed', | ||
| endedAt: Date.now(), |
There was a problem hiding this comment.
Don’t mark session failed after post-initiation errors
The new catch-path update applies to any exception after sessionId is assigned, including failures that happen after Twilio has already accepted the outbound call (for example, an error persisting providerCallSid), so we can persist status: 'failed'/endedAt while the call is actually live. That corrupts lifecycle state and can hide an active call from getActiveCallSessionForConversation, so this failed transition should be limited to genuine initiation failures.
Useful? React with 👍 / 👎.
Summary
setup.shscript #5)Verified
GCP_REGIONwithus-central-1#6 (deterministic ordering), feat: add top-level setup.sh script #7 (pending status enforcement), Upon opening home page, don't auto redirect to creating the assistant #8 (status reset on user answer) were already fixed in prior PRsTest plan