feat(rtc): surface ParticipantActive and Participant.state - #715
Open
tinalenguyen wants to merge 3 commits into
Open
feat(rtc): surface ParticipantActive and Participant.state#715tinalenguyen wants to merge 3 commits into
tinalenguyen wants to merge 3 commits into
Conversation
The FFI already emits a `ParticipantActive` event, but `Room` dropped it on the floor: JS had no way to observe when a remote participant transitions past JOINED, and `Participant` exposed no `state` at all (`info.state` was set once at construction and never refreshed). A remote participant can only receive data messages once it reaches `ParticipantState.ACTIVE`, so callers waiting on `ParticipantConnected` could start sending to a participant that was not yet reachable. The Python SDK has had `participant_active` for this; this brings Node to parity. - handle the `participantActive` FFI event, flip `info.state` to ACTIVE, and emit `RoomEvent.ParticipantActive` - flip `info.state` to DISCONNECTED alongside the existing `disconnectReason` - add the `Participant.state` getter and export `ParticipantState`
🦋 Changeset detectedLatest commit: 09105e4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
`Participant.state` reads `ParticipantInfo.state`, which only ever moved on per-participant events. A room-level disconnect — explicit `disconnect()` or an FFI-driven one — is not reported as each participant departing, so the local participant and every still-listed remote participant kept reporting ACTIVE. That state outlives the disconnect in practice: the participant maps are never cleared, and `Disconnected` handlers routinely capture participants. A retained participant claiming to be ACTIVE reads as reachable when it no longer is. Transition the local participant and all retained remote participants to DISCONNECTED in cleanupOnDisconnect, before `ConnectionStateChanged` and `Disconnected` fire, so a handler that inspects `state` sees the truth. Two remote-participant stubs in audio_stream_room_lifecycle.test.ts gain the `info` that real participants always carry (createRemoteParticipant sets it unconditionally), since cleanup now writes through it. Renames participant_active.test.ts to participant_state.test.ts — it now covers the full state lifecycle rather than just the active transition.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
disconnect() ran cleanupOnDisconnect() outside ffiEventLock and only removed the FfiClient listener afterwards. onFfiEvent is dispatched synchronously but awaits the lock before doing anything, so callbacks delivered before removeListener were still pending when cleanup ran, and went on to process participantActive or participantsUpdated — both of which write participant info — overwriting the DISCONNECTED state cleanup had just set. Remove the listener first so no new callbacks are created, then acquire ffiEventLock before cleanup. The mutex is FIFO, so already-queued callbacks run to completion first and cleanup lands last. This is the JS analogue of the Python SDK's disconnect(), which unsubscribes its queue and awaits the listen task before flipping connection state. cleanupOnDisconnect stays lock-free: the FFI-driven disconnected path reaches it from inside onFfiEvent, which already holds the lock, and Mutex is not reentrant.
| // reaches it from inside onFfiEvent, which already holds it. | ||
| FfiClient.instance.removeListener(FfiClientEvent.FfiEvent, this.onFfiEvent); | ||
|
|
||
| const unlock = await this.ffiEventLock.lock(); |
Contributor
There was a problem hiding this comment.
🔴 Concurrent publishing can hang disconnect
When native disconnection leaves publishTrack pending, disconnect() waits behind its mutex. Cleanup cannot abort the pending operation, so both promises hang.
Suggested change
| const unlock = await this.ffiEventLock.lock(); | |
| this.disconnectController.abort(); | |
| const unlock = await this.ffiEventLock.lock(); |
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The FFI already emits a
ParticipantActiveevent — the proto message is right there inrtc-ffi-bindings(room_pb.d.ts,participant_active = 42) — butRoomdrops it on the floor. Two consequences:JOINED.Participantexposes nostateat all.info.stateis set once increateRemoteParticipantand never refreshed, so it is not a usable substitute.This matters because a remote participant can only receive data messages once it reaches
ParticipantState.ACTIVE. Code that waits onParticipantConnectedgets back a participant that is present inroom.remoteParticipantsbut not yet reachable, and anything sent to it is silently dropped.The Python SDK has had
participant_activefor exactly this (room.py, "Called when a remote participant becomes active and is ready to receive data messages"). This brings Node to parity.Changes
room.tsparticipantActiveFFI event: flipinfo.statetoACTIVEand emit the newRoomEvent.ParticipantActive. Unknown identity warns and no-ops, matching the existingparticipantDisconnectedbranch.info.state = DISCONNECTEDalongside the existingdisconnectReasonassignment, so a departed participant's state isn't left readingACTIVE.participantActiveinRoomCallbacksandParticipantActivein theRoomEventenum.participant.ts— add thestategetter, defaulting toJOININGsinceParticipantInfo.stateis optional in the generated types.index.ts— re-exportParticipantState, next to the existingParticipantKindexport. Without it callers can read.statebut have no enum to compare against.Compatibility
Purely additive — a new event, a new getter, a new export. No existing behavior changes:
ParticipantConnectedfires exactly as before, and nothing that ignores the new event is affected.Tests
Five new tests in
participant_active.test.ts. They drive the privateonFfiEventhandler with the sameroomEventshapeFfiClientdelivers, so the real switch statement is exercised rather than a reimplementation:participantConnectedJOININGdefault when the FFI omits a stateThe
FfiHandlestub is the same oneaudio_stream_room_lifecycle.test.tsuses — fabricated handle ids otherwise trigger a native drop at GC time.pnpm buildsucceeds; 61 tests pass across 6 files; lint is clean on the changed files.Motivation
Follow-up in agents-js depends on this:
waitForParticipantthere resolves onParticipantConnected, soDataStreamAudioOutputcan start streaming avatar audio to a participant that cannot yet receive it. That fix can't be written until this ships.