Add simulation glue#1733
Conversation
🦋 Changeset detectedLatest commit: 9250272 The changes in this PR will be included in the next version bump. This PR includes changesets to release 34 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}
| output: new pb.SessionRequest_UpdateIO_Output({ | ||
| audioEnabled: outputAudioEnabled, | ||
| transcriptionEnabled: outputTranscriptionEnabled, | ||
| }), |
There was a problem hiding this comment.
🟡 updateIo always sends an output sub-message even when no output fields are specified
In RemoteSession.updateIo, the output field is unconditionally constructed as new pb.SessionRequest_UpdateIO_Output(...) even when both outputAudioEnabled and outputTranscriptionEnabled are undefined. This is asymmetric with the input field handling (remote_session.ts:1333-1336), which conditionally creates the object only when inputAudioEnabled is defined. When updateIo({ inputAudioEnabled: true }) is called (only intending to change input), an empty output sub-message is still serialized on the wire. The handler at remote_session.ts:975 checks if (update.output), which will be truthy for the empty sub-message. While the inner field-level !== undefined guards currently prevent unintended changes for proto3 optional fields, if those fields were ever non-optional bool fields, undefined would default to false causing the handler to incorrectly disable audio and transcription output.
| output: new pb.SessionRequest_UpdateIO_Output({ | |
| audioEnabled: outputAudioEnabled, | |
| transcriptionEnabled: outputTranscriptionEnabled, | |
| }), | |
| output: | |
| outputAudioEnabled === undefined && outputTranscriptionEnabled === undefined | |
| ? undefined | |
| : new pb.SessionRequest_UpdateIO_Output({ | |
| audioEnabled: outputAudioEnabled, | |
| transcriptionEnabled: outputTranscriptionEnabled, | |
| }), |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const dynamicRequest = req.request as { case?: string; value?: unknown }; | ||
| if (dynamicRequest.case === 'finalizeSimulation') { | ||
| return this.handleFinalizeSimulation(req.requestId, dynamicRequest.value); | ||
| } |
There was a problem hiding this comment.
🚩 Dynamic proto casting for finalizeSimulation bypasses type safety
Both handleFinalizeSimulation (remote_session.ts:886-888) and the RemoteSession.finalizeSimulation sender (remote_session.ts:1366-1369) use as unknown as casts to inject a finalizeSimulation case into the proto oneof fields. This suggests the proto definition (@livekit/protocol) does not yet include finalizeSimulation in SessionRequest.request or SessionResponse.response. This pattern works at runtime only if the proto serialization library preserves unknown/untyped oneof cases through toBinary()/fromBinary() round-trips — which @bufbuild/protobuf typically does NOT do for unknown oneof variants. If the transport serializes messages (as RoomSessionTransport does via msg.toBinary()), the finalizeSimulation field may be silently dropped. This would only work correctly with TcpSessionTransport or in-memory transports that don't go through protobuf binary serialization. Worth verifying against the actual proto definitions and testing with RoomSessionTransport.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const jobWithMetadata = this.#info.job as proto.Job & { metadata?: string }; | ||
| const roomWithMetadata = this.#room as Room & { metadata?: string }; | ||
| const metadata = jobWithMetadata.metadata || roomWithMetadata.metadata; |
There was a problem hiding this comment.
🚩 simulationContext() uses metadata field accessed via type assertion
At job.ts:199-201, both this.#info.job and this.#room are cast to types with { metadata?: string } to access a metadata property that may not be in the TypeScript type definitions. This implies either (a) the proto Job type's metadata field isn't exposed in the TS bindings yet, or (b) it exists but under a different accessor. The code falls back gracefully (returns undefined if no metadata), and the JSON.parse is wrapped in try-catch, so there's no crash risk. However, if metadata is never actually populated on these objects at runtime, simulationContext() would always return undefined even for simulation jobs.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Testing
Note: pnpm --filter @livekit/agents api:check is currently blocked by the existing API Extractor limitation on export * as syntax in dist/index.d.ts.
Ported from livekit/agents#5688
Original PR description
No description.