Recognize legacy-unencrypted scenario in existing player-scenario dispatch - #115
Merged
Merged
Conversation
The macOS publish workflow has been failing to produce a report at all
for the last two runs: a single case's summary comparison raised an
unhandled KeyError ('audio') deep in _compare_audio_summaries(), which
propagated straight out of asyncio.gather() in run_matrix() and killed
run_all.py before the report/Pages-publish steps ever ran. This is why
recently-added scenarios (e.g. server-initiated-legacy-unencrypted from
#112) appeared to be "missing" from the published site -- the whole
report generation had silently stopped succeeding, not just that one
scenario's row.
- runner.py: wrap _compare_summaries() in run_case() so any exception
it raises is turned into a failed case with a descriptive reason
instead of crashing the entire matrix run. A bug in one scenario's
comparison logic should now only fail that one case.
- _compare_audio_summaries(): guard against a missing/malformed
"audio" section in either summary with an explicit, readable failure
reason instead of a raw KeyError, since that's the concrete bug that
triggered this.
Verified locally: python -m compileall src scripts passes, and a full
aiosendspin -> aiosendspin matrix run (including the new
server-initiated-legacy-unencrypted and
server-initiated-protocol-baseline-v1 scenarios) still produces the
same pass/fail results as before, plus report generation still
succeeds end-to-end.
Co-authored-by: Copilot <[email protected]>
… existing client adapters The client side of this scenario is identical to server-initiated-pcm (same PCM audio, same role behavior) - only the server differs, by skipping the Noise handshake. Rather than requiring new scenario-specific client logic, each client adapter's existing player-scenario dispatch just needed to also match this scenario_id. - sendspin-go: add to IsPlayerScenario - sendspin-cpp: add to is_player_scenario - sendspin-js: add to PLAYER_SCENARIOS
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.
Answers a fair pushback: does every client adapter really need net-new scenario-specific code just to keep testing the plaintext/legacy-unencrypted path that already worked pre-Noise-migration?
No. The client's wire-level behavior for 'server-initiated-legacy-unencrypted' is identical to 'server-initiated-pcm' - same PCM audio format, same role, same verification. The only thing that differs is server-side: whether it performs the Noise handshake or skips it (allow_unencrypted). The client is agnostic to that.
So instead of adding bespoke per-scenario logic, this just adds the new scenario_id string to each adapter's existing player-scenario allow-list (the same pattern already used for server-initiated-flac, server-initiated-opus, etc.):
All three of these already declare supports_legacy_unencrypted=True in implementations.py, and sendspin-go/sendspin-js clients are plaintext-only already (no Noise support at all), making this scenario their native mode. sendspin-cpp's client is likewise unaware of encryption mode.
Not touched: aiosendspin's client (by design - it always uses real Noise and should correctly fail/refuse this scenario), and the currently build-broken adapters (dotnet/jvm/rs/SendspinKit), which are unrelated pre-existing issues.