Repository navigation
Ignore reserved binary IDs and end removed stream roles on server/activate - #436
Merged
chrisuthe merged 5 commits intoSep 17, 2026
Conversation
Without on_legacy_fragment, a type 2 or 3 frame is an ordinary binary message for the owner to ignore, instead of a protocol error. It is still an error while a type 1 fragmented message is in flight. Adds tests for the client ignoring unknown JSON types and unimplemented binary IDs.
A server/activate that removes the player, artwork, visualizer or an application-specific role, explicitly, implicitly or by version replacement, now ends those streams and dispatches the stream end listeners, even when an earlier stream/end already ended the stream.
Server ignores unimplemented client binary IDs, discards old-key messages of unknown type before Noise message 2, and sends stream/end before the re-handshake when pairing on a long-term PSK.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are narrowly scoped, consistent with the stated protocol rules, and backed by comprehensive new/updated tests covering the key edge cases (reserved IDs, reassembly, activation role removal, and re-handshake sequencing).
Pull request overview
This PR advances Sendspin protocol-evolution conformance by making reserved binary IDs (2/3) non-fatal for newer clients and by ensuring the Python client SDK ends streams when server/activate removes stream-capable roles, aligning behavior with the updated receiver rules in Sendspin/spec#289.
Changes:
- Updated
EncryptedWebSocketto deliver reserved binary IDs 2/3 as ordinary binary frames unless the legacy fragment opt-in is enabled (while still rejecting them mid type-1 reassembly). - Updated the client SDK to end streams (and notify stream-end listeners) when
server/activateremoves stream-role families or replaces role versions, including after an earlierstream/end. - Added/extended targeted tests across Noise transport, client SDK behavior, and server dispatch/activation sequencing to lock in the new rules.
File summaries
| File | Description |
|---|---|
| tests/server/test_source_connection_dispatch.py | Adds coverage that unknown client binary IDs are ignored even on strict servers. |
| tests/server/test_role_activation.py | Adds coverage that long-term→pairing re-handshake sequencing puts stream/end on the wire before re-handshake message 1. |
| tests/noise/test_wire.py | Updates transport tests to assert IDs 2/3 are delivered as ordinary binary without legacy opt-in and rejected mid-reassembly. |
| tests/noise/test_driver.py | Extends re-handshake discard test to include unknown JSON type and unknown binary ID in flight. |
| tests/client/test_unrecognized_messages.py | New tests verifying the SDK ignores unknown server JSON types and unimplemented/reserved binary IDs after activation (and closes appropriately post re-handshake). |
| tests/client/test_stream_role_removal.py | New tests verifying stream teardown + listener semantics when server/activate removes/replaces stream roles (including app-specific roles). |
| aiosendspin/noise/wire.py | Implements the reserved-ID delivery change in EncryptedWebSocket while preserving legacy fragment opt-in behavior and mid-reassembly erroring. |
| aiosendspin/client/connection.py | Ends removed-role streams during activation application via _end_removed_role_streams() and factors stream teardown into _end_streams(). |
| aiosendspin/client/client.py | Updates stream-end listener documentation to include role-removal-triggered stream end and required embedder cleanup semantics. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
chrisuthe
marked this pull request as ready for review
September 17, 2026 13:00
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
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.
Part of #399. Implements the remaining receiver rules from Sendspin/spec#289 ("Protocol evolution").
Changes
EncryptedWebSocketnow delivers binary IDs 2 and 3 as ordinary binary messages. Before, it returned an error, which made the client SDK close the connection. The client then ignores them like any other ID it does not implement. A 2/3 frame that arrives while a type 1 fragmented message is being reassembled is still an error. The server always sets the opt-in, so it keeps treating 2/3 as theDEPRECATED(spec-pr-172)legacy fragments.server/activateremovesplayer,artwork,visualizeror an application-specific role, the SDK now ends that role's stream. This covers explicit removal, implicit removal (the connection is no longer playback-capable) and replacement by another role version. Ending the stream marks it inactive, drops its config and blanks any artwork channels on display. The stream end listeners are called once with the removed families, even if an earlierstream/endalready ended the stream.Behaviour change for SDK embedders: the stream end listener now also fires when a role is removed. It fires even if the stream already ended, because the embedder may still be finishing output or holding an effect such as ducking. The listener docs now say the embedder must stop output, clear buffers and release temporary output effects. The SDK can't tell whether an application-specific role carries a stream, so removing one always fires the listener.
Rules
typeafter activationtests/client/test_unrecognized_messages.py::test_unknown_json_type_is_ignored(new)typetests/server/test_client_message_dispatch.py::test_unknown_message_type_keeps_connection_opentests/client/test_unrecognized_messages.py::test_unimplemented_binary_id_is_ignored(new)tests/noise/test_wire.py::test_receive_delivers_reserved_ids_without_legacy_opt_in,tests/client/test_unrecognized_messages.py::test_reserved_binary_id_is_ignoredtests/client/test_unrecognized_messages.py::test_implemented_binary_id_with_inactive_stream_is_dropped(new),tests/test_client_artwork_transfers.py::test_malformed_message_outside_stream_closes_connectiontests/server/test_source_connection_dispatch.py::test_unimplemented_binary_type_is_ignored(new)tests/noise/test_wire.py::test_legacy_fragments_reassemble_with_opt_intests/client/test_unrecognized_messages.py::test_rehandshake_closes_on_anything_but_server_activate(new)tests/noise/test_driver.py::test_rehandshake_discards_old_key_messages_before_message_2(extended with an unknown type and ID)tests/noise/test_wire.py::test_receive_rejects_non_fragment_frame_mid_reassembly,test_receive_rejects_reserved_ids_mid_reassembly_without_legacy_opt_in(new)orig_type(MAY)stream/endbefore a re-handshake whose activation removes a stream roletests/server/test_role_activation.py::test_long_term_pairing_ends_streams_before_rehandshake(new)stream/endtests/server/test_role_activation.py::test_removed_roles_are_torn_down_before_server_activateserver/activatetests/client/test_server_state.py::test_activation_discards_removed_role_statestream/endand on version replacementtests/client/test_stream_role_removal.pyOnly the long-term → pairing re-handshake can remove stream roles, and
_quiesce_for_pairingends the streams and flushes the writer before Noise message 1 goes out.