fix: drain media before BYE on local hangup to prevent voicemail clipping

[rkfshakti]

Fixes livekit/livekit#4737

Problem

When an agent ends a call right after wait_for_playout() returns — EndCall RPC, end_call tool, or removing the SIP participant — the last word of the utterance is clipped on the callee's end (voicemail scenario). The audio is already past the agent: it is buffered in the room mixer (5-frame ~100ms input buffer), the codec encoder, and in-flight RTP. wait_for_playout() only tracks the local queue, so it cannot see this tail.

The SIP bridge then sends BYE and closes media immediately, so whatever is still buffered never clears the wire. A trailing pause masks the bug because it gives the tail time to flush; an abrupt end-of-speech hangup clips the last word. The reporter's workaround (2s pre-hangup sleep) confirms the mechanism.

Fix

Add a configurable drain window hangup_drain_time (default 500ms): on a locally-initiated clean hangup, keep feeding media to the peer for the window before sending BYE. The window is bounded (default 500ms) and only applies to:

• EndCall RPC (reason rpc)
• ctx cancel / agent end-call (reason hangup)
• participant removed from room (reason removed)
• cancelled invite (inbound, reason cancelled)

Remote BYE (the peer hung up), errors, timeouts, media failures, and pre-connect failures are not delayed. Set hangup_drain_time: -1 to disable.

Test

TestOutboundHangupDrainsMediaBeforeBYE: on an established outbound call, an EndCall RPC must not emit BYE before the drain window has elapsed. Fails on the old code (BYE sent before the media drain window elapsed), passes with the fix. All media-port and outbound tests pass; the pre-existing auth-test failures on clean main are unrelated (they fail identically without this change).

[rkfshakti]

Addressed the Devin review finding (cancelled inbound call delayed): drainOnHangup now gates on the media actually being bridged (c.started broken) and "cancelled" is dropped from the local-hangup set — a remote CANCEL before 200 OK is not a local hangup and gets no drain. Also constrained the media-port test range to 10000-20000 so CI can't land on a privileged port (<1024, seen on TestSetOfferReportsUnknownProvider: 'bind: permission denied').

[codecov]

Codecov Report

❌ Patch coverage is 70.00000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.67%. Comparing base (0460b40) to head (fa26463).
⚠️ Report is 370 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/sip/inbound.go	66.66%	4 Missing and 1 partial ⚠️
pkg/config/config.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #811      +/-   ##
==========================================
+ Coverage   65.25%   67.67%   +2.41%     
==========================================
  Files          51       43       -8     
  Lines        6588     8563    +1975     
==========================================
+ Hits         4299     5795    +1496     
- Misses       1915     2242     +327     
- Partials      374      526     +152     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rkfshakti]

Pushed cab53c5 addressing both review comments:

1. Cancellable drain — the drain window is now select { case <-ctx.Done(): case <-time.After(drain): } in both inbound and outbound paths, so context cancellation cuts the drain short instead of a blind time.Sleep.

2. No sleeping under c.mu — outboundCall.close() releases c.mu before the drain wait and re-acquires it after, since Close/CloseWith/CloseWithTimeout all enter with the lock held. A 500ms drain no longer blocks Participant() readers or concurrent shutdown.

Verified: go build ./pkg/sip/ clean, go test ./pkg/sip/ -run 'TestOutbound|Drain|TestInbound' passes; the -race TestService_* failures reproduce on the unmodified head too (pre-existing, unrelated to this PR).

[rkfshakti]

Thanks @genseric-ghiro for the review. I merged the latest main and addressed each thread:

• preserved the original caller context so cancellation/deadlines can cut the optional drain short
• moved the BYE ordering comment next to the actual teardown call
• serialized close ownership without holding c.mu during the drain or while competing closers wait
• added concurrent-hangup regression coverage

The latest head (6bc104d) is mergeable, all checks are green, and all review threads are resolved. Please let me know if anything else is needed — I’m happy to contribute more.

[devin-ai-integration]

Devin Review found 1 new potential issue.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

[devin-ai-integration]

🟡 Pre-connect hangups delay cancellation

Before media starts, drainOnHangup delays local RPC and room-removal terminations. CANCEL then waits despite having no media to drain, extending ringing.

Learn more

Outbound calls set started only after SIP negotiation succeeds and media is connected in connectSIP. A local close can occur before that point, but the new helper classifies it from the termination reason alone. The close path then sleeps before sipOutbound.Close sends the CANCEL needed for an unanswered INVITE.

Example: An EndCall RPC reaches a newly created outbound call before its INVITE is answered. With hangup_drain_time: 2s, the callee keeps ringing for two extra seconds before receiving CANCEL, although no media has started.

Recommended fix: Require c.started.IsBroken() before returning HangupDrainTime, matching the inbound guard.

Suggested change

	if end.Term.Result != stats.ResultSuccess {
	return 0
	}
	if end.Term.Result != stats.ResultSuccess || !c.started.IsBroken() {
	return 0
	}

---

Was this helpful? React with 👍 or 👎 to provide feedback.