feat(agents): read simulator dispatch metadata#1757
feat(agents): read simulator dispatch metadata#1757rosetta-livekit-bot[bot] wants to merge 1 commit into
Conversation
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 waitForParticipant does not filter simulator participants
The existing waitForParticipant method at agents/src/job.ts:285-319 filters out ParticipantKind.AGENT but does not filter out simulator participants (those with the lk.simulator attribute). With this PR introducing simulation support, a simulator participant could potentially be returned by waitForParticipant. This is likely fine if simulator participants use ParticipantKind.AGENT (which would already be filtered), but if they use a different kind, they could be incorrectly returned as the "user" participant. Not flagged as a bug since we can't verify the participant kind used by simulators without access to the server code, and this is pre-existing behavior.
(Refers to lines 285-319)
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (this.#simulationResolved) { | ||
| return this.#simulationContext; | ||
| } | ||
|
|
||
| this.#simulationResolved = true; |
There was a problem hiding this comment.
🔴 simulationContext() permanently caches undefined when called before room is connected or simulator participant has joined
The simulationContext() method at agents/src/job.ts:221-261 sets #simulationResolved = true unconditionally on the first call and caches the result. If the method is called before ctx.connect() (when remoteParticipants is empty) or before the simulator participant has joined the room, the participant attribute lookup at line 229-238 will find nothing. On newer servers where the dispatch is placed in participant attributes (not job metadata), the job-metadata fallback at line 242 will also be empty, causing undefined to be cached permanently. Any subsequent call — even after the room is connected and the simulator participant is present — returns the stale cached undefined, making the simulation context permanently unresolvable.
Prompt for agents
The simulationContext() method in agents/src/job.ts uses a #simulationResolved boolean flag to cache the result after the first call. The problem is it caches even when the result is undefined (no simulation found), which prevents correct resolution if called before the room is connected or before the simulator participant has joined.
Possible approaches:
1. Only set #simulationResolved = true when a SimulationContext is successfully found. When returning undefined, leave #simulationResolved as false so subsequent calls can re-check. This allows the method to be called early (returning undefined) and then called again after connect (finding the actual context).
2. Alternatively, require the room to be connected before caching, e.g. check this.#room.isConnected and only cache when connected.
3. Document that the method must only be called after ctx.connect() and the simulator participant is present, and throw or warn if the room is not connected.
Approach 1 is simplest and most forgiving. The tradeoff is that the method will scan participants on every call until a simulation context is found, but this is a lightweight operation.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| if (!metadata) { | ||
| // Older servers and fake job contexts placed the dispatch in job metadata. | ||
| metadata = (this.#info.job as proto.Job & { metadata?: string }).metadata || ''; |
There was a problem hiding this comment.
🚩 Job metadata fallback uses type assertion for metadata field
At agents/src/job.ts:242, the code casts this.#info.job as proto.Job & { metadata?: string } to access a metadata field. This suggests uncertainty about whether proto.Job from @livekit/protocol@1.46.4 includes a metadata property. If the protocol type already has metadata as a different type (e.g., Uint8Array for protobuf bytes), the || '' fallback with string comparison could behave unexpectedly. If it doesn't have metadata at all, the assertion is necessary but fragile — a future protocol update adding metadata with an incompatible type would silently break. Could not verify the actual proto.Job type definition as node_modules aren't installed.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary\n- add simulator dispatch attribute constants\n- resolve simulation dispatch from the simulator participant's
lk.simulator.dispatchattribute\n- keep job metadata as a fallback for older servers and fake job contexts\n\n## Verification\n-pnpm build:agents\n-pnpm exec prettier --check agents/src/constants.ts agents/src/job.ts\n\nNo tests added; upstream PR #6053 did not add tests.Ported from livekit/agents#6053
Original PR description
No description.