Repository navigation
Send lk.agent.session messages with one-shot byte sends - #2769
rosetta-livekit-bot[bot] wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 02c4deb The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| throw new Error('room session transport is closed'); | ||
| } | ||
|
|
||
| const unlock = await this.sendLock.lock(); |
There was a problem hiding this comment.
🟡 Queued room messages outlive closure
When close() runs while a send waits on sendLock, that send can transmit after closure. The closed check runs before the lock wait, and close() does not cancel queued sends.
Learn more
A room transport serializes outgoing messages with sendLock. A send checks closed before waiting for the lock, but close sets closed without draining or cancelling the queue. A queued call therefore resumes after closure and proceeds to sendFile if the room remains connected. This lets a closing session deliver stale events or responses after its transport has closed.
Example: Send A holds the lock while sendFile waits. Send B passes the initial check and waits for the lock. close() completes, then A finishes; B acquires the lock and sends even though closed is now true.
Recommended fix: Recheck closed and room connectivity after acquiring sendLock and before creating the file. Coordinate close() with any active send if its contract requires no sends after close returns.
Was this helpful? React with 👍 or 👎 to provide feedback.
1egoman
left a comment
1egoman
left a comment
There was a problem hiding this comment.
This is not a good approach. Before this could be merged, a sendBytes method needs to be added to the node sdk as well. I thought I had done this but it seems not, I will move this to draft, make the requisite node sdk changes, and then rework this once there's a node sdk release with sendBytes.
Summary
lk.agent.sessionmessage through rtc-node's one-shot byte-stream path instead of an incrementalstreamByteswriterInfrastructure gap
@livekit/rtc-nodedoes not expose the in-memorysendBytesAPI used by the Python source PR, and its native FFI has no in-memory one-shot byte request. This port therefore uses rtc-node's existing nativesendFileone-shot path with an owner-only temporary file. It preserves the intended data-stream-v2 inline/compression behavior and wire protocol, but adds local filesystem I/O until rtc-node gains an in-memory API.Testing
pnpm test agents/src/voice/remote_session.test.ts(27 passed)pnpm --filter @livekit/agents typecheckpnpm build:agentspnpm --filter @livekit/agents lint(passes with existing warnings)pnpm exec prettier --check "agents/src/**/*.{ts,tsx,md,json}" ".changeset/calm-dodos-send.md"pnpm test agents(3,683 passed, 5 skipped, 3 unrelated failures intelemetry/loop_monitor.test.ts; the container exposes process-level rather than thread-level CPU accounting)Runtime
cue-clivalidation could not be run becausecue-cliis not installed or publicly resolvable in this environment.Source diff coverage
Source PR livekit/agents#7751
livekit-agents/livekit/agents/voice/remote_session.py: adapted toagents/src/voice/remote_session.ts. ReplacedstreamBytes/writer calls with rtc-node's file-backed one-shotsendFile, preserving payload bytes, generated name, topic, destination filtering, failure propagation, and serialized ordering. The file-backed adaptation is required because rtc-node lacks Python's in-memorysendBytesAPI.tests/test_remote_session.py: adapted toagents/src/voice/remote_session.test.ts. Ported the modified concurrent-send coverage, the new assertion thatstreamBytesis not used and each message is sent in one call, exact payload/topic/destination checks, and the modified send-failure behavior.livekit-agents/pyproject.toml: not applicable. This bumps Pythonlivekitto the release that introducedsend_bytes; the target's current@livekit/rtc-nodealready provides the file-backed one-shotsendFileprimitive used by this adaptation, while no rtc-node release provides in-memorysendBytes.uv.lock: not applicable. It only records the Python dependency bump above; no target dependency changed, sopnpm-lock.yamlremains unchanged.Source: livekit/agents#7751
Ported from livekit/agents#7751
Original PR description
Summary
RoomSessionTransport.send_messagenow sends eachlk.agent.sessionmessage with the one-shotLocalParticipant.send_bytesinstead of opening an incrementalstream_bytesstream per message.With
stream_bytes, an incremental stream always results in three packets being sent on the reliable data channel (header, chunk, trailer) and is never compressed.With
send_bytesthe SDK knows the whole payload up front, so for receivers that support data streams v2 it can send a small message as a single inline packet, and compress a large one.Why it matters
Session events are small and frequent, and today they go to every participant in the room, because the room session host is created with no remote identity. Measured against a JS client (
clientProtocolof 3), one agent turn sent ~95 session messages: 89×session_usage_updated, 4×agent_state_changed, 2×conversation_item_added. On the client's data channel:stream_bytes(today)send_bytes(this PR)On constrained mobile links, that traffic queues ahead of RPCs and transcriptions on the reliable channel. That's the head-of-line blocking behind customer reports of agent → client RPC connection timeouts.
TODO
LocalParticipant.send_bytesbeing merged and released inlivekit-rtc(livekit/python-sdks PR: feat(rtc): add LocalParticipant.send_bytes python-sdks#858)livekit-rtcpackage dependency