Skip to content

test(interviews): exercise the voice relay against a real WebSocket - #151

Merged
L4XB merged 1 commit into
mainfrom
test/voice-relay-transport
Sep 15, 2026
Merged

L4XB merged 1 commit into
mainfrom
test/voice-relay-transport

Conversation

@L4XB

@L4XB L4XB commented Sep 15, 2026

Copy link
Copy Markdown
Member

Summary

Nine tests that run the relay against a websockets server on 127.0.0.1,
through the real _connect_provider transport — NoRedirectConnect subclass
included. No provider, no credential, no outbound connection, no opt-in marker.
Seven seconds.

Three of the four failure modes the issue names behave as they should. Two
findings came out of the other one, and out of the close codes. Both are pinned
as they behave today and filed separately.

What the tests establish

Test What it pins
test_a_whole_turn_survives_the_real_transport setup → audio → activityEnd → transcript + meter → participant stop, over a socket. The provider is configured by the relay, never by the browser.
test_a_provider_that_closes_mid_stream_ends_the_participant_session A clean close upstream becomes reason="connection", relayEnd, and a closed browser socket — not a hang.
test_an_unexpected_close_code_still_ends_the_session (1011, 1008) Abnormal closes end promptly and tell the participant.
test_a_connection_dropped_without_a_close_frame_ends_the_session Same, with the socket aborted and no close handshake at all.
test_a_participant_cannot_outrun_the_ingress_allowance The token bucket and bounded queue still hold with a socket upstream; overrunning them ends the session cleanly.
test_a_provider_that_stops_reading_is_only_noticed_at_the_session_deadline The session does end, and the participant is told.
test_the_api_key_is_never_re_sent_to_a_redirect_target The redirect target records no request at all, so no header, so no key.
test_a_provider_frame_above_the_transport_limit_ends_the_session An oversized frame ends the session rather than being parsed.

The connector asserts it was handed PROVIDER_WS_URL before substituting the
local port, so a change that stopped dialling the provider would fail these
tests rather than pass them quietly.

Two findings, filed rather than fixed here

Both are pinned exactly as they behave today, with a comment in the test naming
the issue. Landing a test and a behaviour change together makes it impossible to
tell which broke something later — the project's own argument, from #140.

#149 — a provider dropping the socket is reported as an internal failure

What the provider did reason diagnostic_code
close frame, code 1000 connection connection
close frame, code 1011 unavailable internal_failure
close frame, code 1008 unavailable internal_failure
socket aborted, no close frame unavailable internal_failure
handshake answered with a redirect unavailable internal_failure

Only a graceful close reaches run_provider's own
RelayPolicyError("connection"). Everything else raises a websockets
exception, which is not a RelayPolicyError, so it lands in the catch-all in
_Relay.run that exists for genuine faults in the deployment. The redaction
that block performs is doing its job; the classification is not. The more
clearly the provider misbehaves, the more confidently an operator is sent
looking for a bug in their own service.

#150 — a provider that stops answering is only noticed at the deadline

A provider that answers setupComplete and then stops reading and stops
answering is not detected while the participant is speaking. Every watchdog
timeout needs silence (patience_ms), a started generation
(generation_timeout_seconds), a completed one (meter_timeout_seconds) or an
idle phase (silence_timeout_seconds). A speaking participant keeps refreshing
last_voice, so none of them applies and the only limit left is the session
deadline.

Nor does the transport push back in time to matter. Measured at ten times real
time while writing this, the relay handed 8.5 MB across twenty-two seconds
into a socket nobody was draining, and send never blocked once
. Loopback
buffers are generous; a thirty-minute session at real time is the same shape,
and the memory sits in a process serving other interviews.

Keepalive does not cover it either: ping_interval=20, ping_timeout=20 proves
the peer process is reachable, because pongs come from the library below the
application — not that anyone is listening.

Behavior and compatibility

Tests only. No source file changed. COMMUNITY_EXPORT_MANIFEST.json records the
new file, as any change under services/api/ must.

Validation

$ uv run --frozen --no-sync pytest -q tests/test_voice_relay_transport.py
  9 passed in 7.01s
$ # three consecutive runs: 6.95s, 6.96s, 6.96s — no flakiness observed
$ uv run --frozen --no-sync pytest -q
  2913 passed, 1 skipped   (2904 before)
$ uv run --frozen --no-sync ruff check src scripts tests   # All checks passed!
$ uv run --frozen --no-sync mypy src                       # no issues found in 208 source files
$ python scripts/audit_community_export.py services/api --refresh-manifest
  community export manifest refreshed: added:tests/test_voice_relay_transport.py
$ python scripts/audit_community_export.py services/api
  community export audit passed: 406 files, 400 HTTP operations, 1 WebSocket, 89 tables

Notes on making it fast and deterministic:

  • The ingress burst is queued before the relay starts, so the outcome does
    not depend on how promptly a loaded runner wakes a sleeping task. Forty frames
    is 128 kB against 32 kB plus one second of refill; no scheduling delay makes
    that fit.
  • The deadline test shortens deadline_at to 1.5 s, which is the only way to
    assert a bounded outcome for a gap whose real bound is LIVE_SESSION_MAX_MINUTES.
  • The local server gets close_timeout=1. That is the harness's patience, not
    the relay's: a provider that has stopped reading never processes the relay's
    close frame, and waiting the library's default ten seconds for a handshake
    that cannot complete cost ten seconds per run for nothing.

Async scenarios use asyncio.run inside synchronous tests, as
test_voice_relay.py already does, so no test-runner plugin and no new
dependency.

Not covered, stated rather than implied

Keepalive timing. ping_interval=20, ping_timeout=20 means exercising it costs
at least twenty seconds of wall clock, which is more than the property is worth
in a suite this size — and, per #150, a pong proves less than it appears to.

  • Engine checks pass, or the engine is unaffected. — unaffected
  • API, migration, worker, and web-contract checks pass.
  • Web type-check, tests, and production build pass, or the web app is unaffected. — unaffected
  • Browser-extension contracts and a deployment-bound build pass, or the extension is unaffected. — unaffected
  • macOS Companion boundary check, locked resolution, tests, and release build pass on the pinned Xcode toolchains—or the Companion is unaffected. — unaffected
  • Self-hosting tests and container builds pass, or deployment is unaffected. — unaffected
  • User-facing behavior has a focused test or the omission is explained. — no behaviour changed; this is the test

Review boundaries

  • Security and privacy effects were reviewed. Every socket binds to
    127.0.0.1 on a kernel-assigned port and is torn down with the test. The
    only credential is the literal string transport-test-key-not-real. The
    redirect test asserts the target recorded no request, which is a
    stronger statement than "no key in the headers it received". No
    participant content, no fixture derived from real audio.
  • New network calls and processors are operator-configurable, fail closed, and document data egress, cost, retention, and failure behavior—or none were added. — none; loopback only
  • Dependencies and bundled assets are justified, locked, and redistribution-compatible—or none were added. — none. websockets is already a runtime dependency; its server side is used here, not added.
  • Native-client changes include explicit origin, local-retention/deletion, permission, signing, update, and binary-distribution implications. — none
  • Research-method assumptions, limitations, and provenance remain visible—or
    no research-facing behavior changed. Nothing changed. The two findings are
    recorded in voice: a provider dropping the socket is reported as an internal failure of the deployment #149 and voice: a provider that stops answering is only noticed at the session deadline #150 rather than left in a comment.
  • Accessibility and keyboard behavior were reviewed for UI changes—or no UI changed. — no UI
  • Browser permissions, capture bounds, pairing callbacks, extension storage, and generated host access were reviewed—or the extension is unaffected. — unaffected

Source-release hygiene

  • No secret, private key, production configuration, customer/participant data, user upload, database dump, log, private prompt, or non-redistributable research content is included.
  • The change belongs in the community stack; payment, subscription, commercial-plan, hosted-administration, and marketing-site code remains separate.
  • Public behavior and limitations are documented — including the two limitations found.
  • CHANGELOG.md is updated for user-visible changes, or the omission is explained above: no source file changed, so there is nothing for a user of a release to read. The two findings are user-relevant and are recorded as issues.
  • Every commit carries my own matching DCO Signed-off-by trailer.
  • I have read CLA.md and posted its exact acceptance sentence as a standalone pull-request comment.
  • I have read and will follow the Code of Conduct.

Visual evidence

No UI change.

Closes #139

The relay was covered thoroughly at the policy layer and not at all at the
transport. Nothing in the suite opened a socket, so framing, close codes and
backpressure rested on the code looking correct — which #138 made concrete by
taking websockets from 16.1.1 to 17.1 on the library carrying live interview
audio.

Nine tests now run the relay against a websockets server on 127.0.0.1, through
the real _connect_provider transport including its NoRedirectConnect subclass.
No credential, no egress, no opt-in marker, seven seconds.

Three of the four failure modes the issue names behave as they should. A whole
turn survives the real transport. A provider closing cleanly mid-stream gives
the participant a defined end rather than a hang. A participant cannot outrun
the ingress allowance. A redirect is refused end to end, and the target server
records no request at all, so the API key cannot have been re-sent.

Two findings, both pinned as they behave today and filed rather than changed
here, because a test and a behaviour change landing together makes it impossible
to tell which broke something later.

An abnormal close, a dropped connection and a refused redirect are all reported
as internal_failure. Only a graceful close reaches run_provider's own
RelayPolicyError; every other transport failure raises a websockets exception
and falls through to the catch-all, so the more clearly the provider misbehaves,
the more confidently the operator is blamed. See #149.

A provider that answers setup and then stops reading is not noticed while the
participant is speaking. Every watchdog timeout needs silence, a started
generation, or an idle phase; a speaking participant keeps refreshing last_voice
and none of them applies. At ten times real time this handed 8.5 MB to a socket
nobody was draining without send ever blocking. The session still ends at its
deadline, which is what the test asserts. See #150.

Closes #139

Signed-off-by: L4XB <L4XB@users.noreply.github.com>
@L4XB

L4XB commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

I have read and agree to the SixSentences CLA v1.0.

@L4XB

L4XB commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

Self-review

Recorded under the founding-maintainer exception in GOVERNANCE.md. One new
file and one regenerated manifest; no source changed.

services/api/COMMUNITY_EXPORT_MANIFEST.json — a single added entry,
tests/test_voice_relay_transport.py. Nothing else moved, which is the point of
reading it: no file entered or left the audited tree as a side effect. The audit
then reports 406 files where it reported 405.

services/api/tests/test_voice_relay_transport.py — read in full against
the relay it exercises.

The harness.

  • Browser duplicates the stub in test_voice_relay.py rather than importing
    it. Deliberate: cross-importing between test modules makes a change to one
    file fail the other for reasons its own diff does not show. The duplication is
    about twenty lines and both copies are simple.
  • drive's connector asserts it was handed PROVIDER_WS_URL before
    substituting the local port. Without that assertion a change that stopped
    dialling the provider would make these tests pass more easily, not less.
  • local_provider sets close_timeout=1. Checked that this is the harness's
    patience and not the relay's: _connect_provider keeps its own
    close_timeout=3, and _close_and_persist keeps its
    wait_for(..., timeout=3). Without it the deaf-provider test spent ten
    seconds waiting for a close handshake that cannot complete, because the peer
    that stopped reading never processes the close frame.
  • until() replaced a sleep(0.8) in the happy path. The first draft guessed a
    duration and raced patience_ms; waiting for the turn to actually complete is
    both faster and deterministic.

Determinism. Each test owns its session_id, so _ACTIVE_RELAYS cannot
collide if the file is ever run in parallel. Ports come from the kernel. The
ingress burst is queued before the relay starts, so a loaded runner cannot turn
a flood into a legal rate. Ran three times back to back: 6.95 s, 6.96 s, 6.96 s.

Cost. 7 s of a suite that takes five minutes locally. The deaf-provider test
is 5.5 s of that, and it is the one that found #150.

The two pinned defects. assert run.terminal.diagnostic_code == "internal_failure"
asserts something I have argued in #149 is wrong. That is intentional and the
comment above it says so. The alternative — fixing it here — would land a
behaviour change and its test together, and the project's argument against that
in #140 applies to its own code too. Whoever fixes #149 will find these
assertions and update them, which is the desired outcome.

One thing I checked and left alone. test_a_provider_frame_above_the_transport_limit_ends_the_session
asserts reason in {"connection", "unavailable"} rather than one value.
_connect_provider caps max_size at 1 MiB and handle_provider_message caps
max_provider_message_bytes at the same number, so which layer refuses first is
a race I did not want to pin — the property under test is that an oversized
frame is not parsed, and both outcomes satisfy it. Narrowing it would assert an
implementation detail of websockets.

One thing this pull request confirms about another

Publish CLA status is green here. This is the first pull request opened
after #148 landed, opened and then immediately given its CLA acceptance comment
— the exact sequence that produced a failed check on #144, #147 and #148. The
end-to-end criterion I said in #148 could only be observed on a later pull
request is met.

Checks

Every context green, including the API job that ran the new tests, the
self-hosting container builds, and Swift.

Merging.

@L4XB
L4XB merged commit fd9e6c2 into main Sep 15, 2026
23 checks passed
@L4XB
L4XB deleted the test/voice-relay-transport branch September 15, 2026 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

voice: nothing exercises the relay against a real WebSocket

1 participant