Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions pkg/sip/attrs_headers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
// Copyright 2026 LiveKit, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package sip

import (
"testing"

"github.com/stretchr/testify/require"
)

// Regression for livekit/sip#404: when the agent deletes the room before SIP
// sends BYE, LocalParticipant is gone. attributes_to_headers must still map
// from the last cached participant attributes.
func TestFillHeadersUsesCachedAttrsWhenRoomNil(t *testing.T) {
call := &inboundCall{
attrsToHdr: map[string]string{
"sip.custom": "X-Custom-Header",
},
}
call.storeParticipantAttrs(map[string]string{
"sip.custom": "value-from-cache",
"other": "ignored",
})
call.lkRoom = nil
cc := &sipInbound{call: call}

headers := cc.fillHeaders(nil)
require.Equal(t, map[string]string{"X-Custom-Header": "value-from-cache"}, headers)

// No mapping configured → leave headers untouched.
call.attrsToHdr = nil
require.Nil(t, cc.fillHeaders(nil))

// Mapping configured but cache empty → leave headers untouched.
call.attrsToHdr = map[string]string{"sip.custom": "X-Custom-Header"}
call.attrsMu.Lock()
call.cachedAttrs = nil
call.attrsMu.Unlock()
require.Nil(t, cc.fillHeaders(nil))
}

func TestOutboundSetAttrsToHeadersUsesCachedAttrsWhenRoomNil(t *testing.T) {
call := &outboundCall{
sipConf: sipOutboundConfig{
attrsToHeaders: map[string]string{
"sip.custom": "X-Custom-Header",
},
},
}
call.storeParticipantAttrs(map[string]string{
"sip.custom": "outbound-cache",
})
call.lkRoom = nil

headers := call.setAttrsToHeaders(nil)
require.Equal(t, map[string]string{"X-Custom-Header": "outbound-cache"}, headers)
}

func TestAttrsToHeaders(t *testing.T) {
attrs := map[string]string{"a": "1", "b": "2"}
mapping := map[string]string{"a": "X-A", "missing": "X-Missing"}
headers := AttrsToHeaders(attrs, mapping, map[string]string{"Keep": "yes"})
require.Equal(t, map[string]string{
"Keep": "yes",
"X-A": "1",
}, headers)
}

// snapshotParticipantAttrs skips a read that returns no attributes, so a
// teardown-time empty read does not wipe the cached values that BYE's
// attributes_to_headers relies on (livekit/sip#404). This is exercised on
// the lkRoom == nil path here: the function returns early without touching
// the cache. The "room up, attributes empty" path is the same guard, but
// needs a live lksdk participant so it is left to integration.
func TestSnapshotParticipantAttrsDoesNotWipeCacheOnEmptyRead(t *testing.T) {
for _, setup := range []func() *inboundCall{
func() *inboundCall {
c := &inboundCall{}
c.storeParticipantAttrs(map[string]string{"sip.custom": "seeded"})
c.snapshotParticipantAttrs() // lkRoom nil → early return, cache intact
return c
},
} {
c := setup()
c.attrsMu.Lock()
got := c.cachedAttrs
c.attrsMu.Unlock()
require.Equal(t, map[string]string{"sip.custom": "seeded"}, got,
"empty/nil room read must not wipe cached attrs")
}
}

func TestOutboundSnapshotParticipantAttrsDoesNotWipeCacheOnEmptyRead(t *testing.T) {
c := &outboundCall{}
c.storeParticipantAttrs(map[string]string{"sip.custom": "outbound-seeded"})
c.snapshotParticipantAttrs() // lkRoom nil → early return, cache intact
c.attrsMu.Lock()
got := c.cachedAttrs
c.attrsMu.Unlock()
require.Equal(t, map[string]string{"sip.custom": "outbound-seeded"}, got,
"empty/nil room read must not wipe cached attrs")
}
59 changes: 53 additions & 6 deletions pkg/sip/inbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,8 @@ type inboundCall struct {
callStart time.Time
extraAttrs map[string]string
attrsToHdr map[string]string
attrsMu sync.Mutex
cachedAttrs map[string]string // last-seen participant attrs for BYE/REFER after room teardown (#404)
ctx context.Context
cancel func()
closeReason atomic.Pointer[ReasonHeader]
Expand Down Expand Up @@ -1578,10 +1580,11 @@ func (c *inboundCall) close(ctx context.Context, end EndCall) {
defer log.Infow("Inbound call closed")
}

// Snapshot attrs before teardown. Prefer live room state, but keep the
// cache so attributes_to_headers still works when the agent deleted the
// room first (Room() is already nil). See livekit/sip#404.
c.snapshotParticipantAttrs()
// Send BYE _before_ closing media/room connection.
// This ensures participant attributes are still available for
// attributes_to_headers mapping in the setHeaders callback.
// See: https://github.com/livekit/sip/issues/404
c.cc.CloseWithStatus(ctx, result, end.Headers)
c.closeMedia()
if callDurFn := c.callDur; callDurFn != nil {
Expand Down Expand Up @@ -1732,6 +1735,7 @@ func (c *inboundCall) setStatus(v CallStatus) {
r.LocalParticipant.SetAttributes(map[string]string{
livekit.AttrSIPCallStatus: attr,
})
c.snapshotParticipantAttrs()
}

func (c *inboundCall) createLiveKitParticipant(ctx context.Context, rconf RoomConfig, status CallStatus) error {
Expand Down Expand Up @@ -1764,6 +1768,10 @@ func (c *inboundCall) createLiveKitParticipant(ctx context.Context, rconf RoomCo
if err != nil {
return err
}
// Seed attrs cache from the join config so BYE mapping works even if the
// room is torn down before we read LocalParticipant again (#404).
c.storeParticipantAttrs(partConf.Attributes)
c.snapshotParticipantAttrs()
if err := registerSignalingRPC(c.lkRoom, c.cc); err != nil {
return err
}
Expand Down Expand Up @@ -2016,11 +2024,50 @@ func (c *sipInbound) fillHeaders(headers map[string]string) map[string]string {
if c == nil || c.call == nil || len(c.call.attrsToHdr) == 0 {
return headers
}
r := c.call.lkRoom.Room()
if r == nil {
attrs := c.call.participantAttributes()
if len(attrs) == 0 {
return headers
}
return AttrsToHeaders(r.LocalParticipant.Attributes(), c.call.attrsToHdr, headers)
return AttrsToHeaders(attrs, c.call.attrsToHdr, headers)
}

// snapshotParticipantAttrs caches LocalParticipant attributes while the room
// is still connected. Used so BYE/REFER can map attributes_to_headers after
// the room has already been torn down (livekit/sip#404).
func (c *inboundCall) snapshotParticipantAttrs() {
if c == nil || c.lkRoom == nil {
return
}
r := c.lkRoom.Room()
if r == nil || r.LocalParticipant == nil {
return
}
attrs := r.LocalParticipant.Attributes() // clones
if len(attrs) == 0 {
// An empty read (room or participant already torn down) must not
// wipe the last known attributes: they back attributes_to_headers
// on BYE/REFER (livekit/sip#404).
return
Comment on lines +2046 to +2050

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Cleared attributes emit stale headers

When a live participant clears every attribute, snapshotParticipantAttrs preserves the old cache. BYE and REFER then emit headers for nonexistent attributes.

Learn more

An empty attribute map is also a valid participant state. These guards cannot distinguish that state from a teardown-time empty SDK read, so both inbound and outbound snapshots retain values that the participant removed. Header generation later treats those retained values as current.

Example: A participant starts with sip.custom=seeded, then removes every attribute while the room remains connected. Its BYE is expected to omit the mapped X-Custom header, but the cache still supplies seeded.

Recommended fix: Track confirmed attribute updates separately from teardown reads. Record confirmed empty states, while preserving the cache only when room lifecycle state proves the participant is unavailable or the read is transient.

Devin Review


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

}
c.attrsMu.Lock()
c.cachedAttrs = attrs
c.attrsMu.Unlock()
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
}

func (c *inboundCall) storeParticipantAttrs(attrs map[string]string) {
if c == nil || len(attrs) == 0 {
return
}
c.attrsMu.Lock()
c.cachedAttrs = maps.Clone(attrs)
c.attrsMu.Unlock()
}

func (c *inboundCall) participantAttributes() map[string]string {
c.snapshotParticipantAttrs()
c.attrsMu.Lock()
defer c.attrsMu.Unlock()
return maps.Clone(c.cachedAttrs)
}

func (c *sipInbound) Drop() {
Expand Down
64 changes: 53 additions & 11 deletions pkg/sip/outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,13 @@ type outboundCall struct {
jitterBuf bool
projectID string

mu sync.RWMutex
mon *stats.CallMonitor
lkRoom RoomInterface
lkRoomIn msdk.PCM16Writer // output to room; OPUS at 48k
sipConf sipOutboundConfig
mu sync.RWMutex
mon *stats.CallMonitor
lkRoom RoomInterface
lkRoomIn msdk.PCM16Writer // output to room; OPUS at 48k
sipConf sipOutboundConfig
attrsMu sync.Mutex
cachedAttrs map[string]string // last-seen participant attrs for BYE after room teardown (#404)
}

func (c *Client) newCall(ctx context.Context, tid traceid.ID, conf *config.Config, log logger.Logger, id LocalTag, room RoomConfig, sipConf sipOutboundConfig, state *CallState, projectID string) (*outboundCall, error) {
Expand Down Expand Up @@ -167,11 +169,47 @@ func (c *outboundCall) setAttrsToHeaders(headers map[string]string) map[string]s
if len(c.sipConf.attrsToHeaders) == 0 {
return headers
}
r := c.lkRoom.Room()
if r == nil {
attrs := c.participantAttributes()
if len(attrs) == 0 {
return headers
}
return AttrsToHeaders(r.LocalParticipant.Attributes(), c.sipConf.attrsToHeaders, headers)
return AttrsToHeaders(attrs, c.sipConf.attrsToHeaders, headers)
}

func (c *outboundCall) snapshotParticipantAttrs() {
if c == nil || c.lkRoom == nil {
return
}
r := c.lkRoom.Room()
if r == nil || r.LocalParticipant == nil {
return
}
attrs := r.LocalParticipant.Attributes() // clones
if len(attrs) == 0 {
// An empty read (room or participant already torn down) must not
// wipe the last known attributes: they back attributes_to_headers
// on BYE (livekit/sip#404).
return
}
c.attrsMu.Lock()
c.cachedAttrs = attrs
c.attrsMu.Unlock()
}

func (c *outboundCall) storeParticipantAttrs(attrs map[string]string) {
if c == nil || len(attrs) == 0 {
return
}
c.attrsMu.Lock()
c.cachedAttrs = maps.Clone(attrs)
c.attrsMu.Unlock()
}

func (c *outboundCall) participantAttributes() map[string]string {
c.snapshotParticipantAttrs()
c.attrsMu.Lock()
defer c.attrsMu.Unlock()
return maps.Clone(c.cachedAttrs)
}

func (c *outboundCall) ensureClosed(ctx context.Context) {
Expand Down Expand Up @@ -371,10 +409,10 @@ func (c *outboundCall) close(ctx context.Context, end EndCall) bool {
info.DisconnectReason = end.Reason
})

// Snapshot attrs before teardown so attributes_to_headers still works
// when the room was already deleted (livekit/sip#404).
c.snapshotParticipantAttrs()
// Send BYE _before_ closing media/room connection.
// This ensures participant attributes are still available for
// attributes_to_headers mapping in the setHeaders callback.
// See: https://github.com/livekit/sip/issues/404
c.stopSIP(ctx, end.Term, end.Headers)
if c.media != nil {
c.media.Close()
Expand Down Expand Up @@ -474,6 +512,8 @@ func (c *outboundCall) connectToRoom(ctx context.Context, lkNew RoomConfig, getR
}
c.lkRoom = r
c.lkRoomIn = local
c.storeParticipantAttrs(attrs)
c.snapshotParticipantAttrs()
if err := registerSignalingRPC(c.lkRoom, c.cc); err != nil {
return err
}
Expand Down Expand Up @@ -672,6 +712,7 @@ func (c *outboundCall) setStatus(v CallStatus) {
r.LocalParticipant.SetAttributes(map[string]string{
livekit.AttrSIPCallStatus: attr,
})
c.snapshotParticipantAttrs()
}

func (c *outboundCall) setExtraAttrs(hdrToAttr map[string]string, opts livekit.SIPHeaderOptions, cc Signaling, hdrs Headers) {
Expand All @@ -680,6 +721,7 @@ func (c *outboundCall) setExtraAttrs(hdrToAttr map[string]string, opts livekit.S
room := c.lkRoom.Room()
if room != nil {
room.LocalParticipant.SetAttributes(extra)
c.snapshotParticipantAttrs()
} else {
c.log.Warnw("could not set attributes on nil room", nil, "attrs", extra)
}
Expand Down
Loading