Repository navigation
test: split websocket client tests into one class per concern - #871
Draft
AlexanderZ-Band wants to merge 6 commits into
Draft
AlexanderZ-Band wants to merge 6 commits into
AlexanderZ-Band wants to merge 6 commits into
Conversation
AlexanderZ-Band
force-pushed
the
feat/fix-agent-stays-offline-forever-after-websocket-cl-INT-1808
branch
from
October 19, 2026 19:03
cf78d3c to
f3cfd6b
Compare
A live agent stopped for good after a received close 1000 or 1001: the
SDK inherited the phoenix client's default ReconnectPolicy, which reads
those codes as an intentional stop. On Band a close code is never the
stop signal. The platform's "disconnect" broadcast closes with 1001
("reconnect expected"), and it answers any close the client or a proxy
starts with 1000. The stop signals are a supersede push and an upgrade
rejection.
- State Band's rule where the SDK builds its transport:
RECONNECT_POLICY = ReconnectPolicy(reconnect_on_normal_close=True).
- A terminal supersede now ends the transport (it starts
PHXChannelsClient.shutdown) instead of only clearing auto_reconnect.
The transport reads that flag once per close, before its backoff, so a
supersede handled during the backoff would otherwise reconnect and
evict the newer same-key instance. Waiters join the transport's own
shielded shutdown, so a host cancelling run_forever never aborts it,
and run_forever returns once it settles.
- band.testing gains close_transport(link), a client-started normal
close, and FakePhoenixServer.stalled(), a slow peer; docstrings that
stated the old rule are rewritten.
- A Hypothesis model test drives generated close, control, supersede and
host-cancel sequences against the link's lifecycle invariants;
hypothesis joins the dev extras.
Behavior changes:
- Agents now reconnect after close 1000/1001, whoever started it.
- An agent whose key is disabled retries a refused (403) reconnect with
backoff until the SDK adopts the phoenix-channels-python-client
release that stops on a refused handshake. It used to stop silently.
- On-prem platforms older than 0.9.1 send no supersede push, so
same-key duplicates there would keep displacing each other; upgrade
those installs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBt2TLCc2YD1AXxmmo61Hx
A supersede pushed behind the control-channel join was handled before BandLink assigned its client, so the handler skipped ending the transport and the closing 1001 reconnected, evicting the newer same-key instance. The handler now receives the client the push arrived on. A WebSocketClient entered again after a terminal supersede kept its "transport ended" marker, so run_forever shut the new healthy transport down at once. The marker is reset with the other per-lifecycle state, and the shutdown task is owned so a failure is logged instead of lost. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MaoYFzCV2bJRD4sDDVD7td
The model test waited out the transport's real reconnect backoff for 22-28 s of the 30 s timeout and failed in the full suite. It now runs with a millisecond backoff, which also lifts the one-reconnect-per-sequence limit. A step releases the slow control signal and requires every signal applied in push order, so a broken control lane fails it. The transport helper test drives a real abort, and the live supersede and reconnect cells follow the one-class-per-file rule. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MaoYFzCV2bJRD4sDDVD7td
AlexanderZ-Band
force-pushed
the
feat/fix-agent-stays-offline-forever-after-websocket-cl-INT-1808
branch
from
October 20, 2026 03:59
337e4b7 to
7c6c550
Compare
- The cancelled-run test now asserts a later run_forever returns, which an unshielded await of the shared shutdown breaks. - The lifecycle model bounds each step and skips shrinking, so a regression fails in seconds rather than minutes; an explicit example pins ordering. - Every class that reconnects uses the millisecond backoff fixture, now in the shared conftest, so no test waits out the transport's real timers. - The auto_reconnect race guard moves to its own dependency-guard class. - The live reconnect smoke is one method per drop mode, so both cells are keyed to their adapter in the manifest. - live_connection is exported from band.testing; comments on the end marker, the end_transport guard and the reconnect loop say what holds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PBt2TLCc2YD1AXxmmo61Hx
- run_forever waits on the scheduled shutdown task through report_owned instead of re-entering the phoenix client's shutdown. - connect() leaves the link disconnected when a supersede handled during the join has already ended the transport. - disconnect() skips leaving agent_control once the transport has ended, so a superseded agent's shutdown logs no spurious warning. - The lifecycle model waits until a signal queued behind a held one is on the lane, so its ordering invariant fails every time ordering breaks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PBt2TLCc2YD1AXxmmo61Hx
tests/websocket/test_client.py held 53 module-level tests across six concerns. They move, unchanged apart from self and two helper renames, into the tests/websocket/client/ package: a kit.py for the shared payloads, dispatch, upgrade-error builders and in-process Phoenix peer, and one class each for payload validation, event routing, wire vocabulary, upgrade errors, initial connect and the watchdog over a real wire. The 58 collected test ids are identical before and after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PBt2TLCc2YD1AXxmmo61Hx
AlexanderZ-Band
force-pushed
the
refactor/split-websocket-client-tests-INT-1808
branch
from
October 20, 2026 05:54
42c04e1 to
94cd01c
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Client reuse and supersede handling during connection setup retain unresolved lifecycle defects.
No open issues 🎉
What changed in this PR
Reorganizes WebSocket client tests into concern-specific classes. The supplied diff also includes stacked reconnect and control-handling changes from #869, beyond the test-only scope described.
Changes:
- Splits client tests into six focused modules with shared helpers.
- Adds normal-close reconnect and terminal-session handling.
- Expands lifecycle, scenario, and live-platform coverage.
| File | Description |
|---|---|
uv.lock |
Locks Hypothesis dependencies. |
tests/websocket/test_watchdog.py |
Updates a test-location docstring. |
tests/websocket/test_client.py |
Removes the monolithic test module. |
tests/websocket/client/test_wire_vocabulary.py |
Groups wire-vocabulary checks. |
tests/websocket/client/test_watchdog_real_wire.py |
Groups real-wire watchdog tests. |
tests/websocket/client/test_upgrade_errors.py |
Groups upgrade and credential tests. |
tests/websocket/client/test_payload_validation.py |
Groups payload-validation tests. |
tests/websocket/client/test_initial_connect.py |
Groups initial-connection tests. |
tests/websocket/client/test_event_routing.py |
Groups event-routing tests. |
tests/websocket/client/kit.py |
Centralizes shared test helpers. |
tests/websocket/client/__init__.py |
Establishes the test package. |
tests/testing/test_transport.py |
Groups transport-helper tests into a class. |
tests/testing/test_phoenix_server.py |
Revises close and reconnect coverage. |
tests/scenarios/world.py |
Adds session-lifecycle scenario helpers. |
tests/scenarios/test_session_end_during_control.py |
Tests shutdown during blocked controls. |
tests/scenarios/script.py |
Adds explicit control-handler gates. |
tests/platform/test_link_credentials.py |
Updates a test-location docstring. |
tests/platform/test_link_control.py |
Awaits asynchronous control delivery. |
tests/platform/test_link_control_lane.py |
Tests ordered control processing. |
tests/platform/linkclose/test_supersede_ends_session.py |
Tests terminal supersede behavior. |
tests/platform/linkclose/test_supersede_during_control.py |
Tests supersede during blocked control handling. |
tests/platform/linkclose/test_lifecycle_model.py |
Checks generated lifecycle sequences. |
tests/platform/linkclose/test_close_reconnect.py |
Tests normal-close reconnect behavior. |
tests/platform/linkclose/lifecycle.py |
Implements lifecycle-model helpers. |
tests/platform/linkclose/kit.py |
Shares connection-lifecycle helpers. |
tests/platform/linkclose/__init__.py |
Establishes the lifecycle-test package. |
tests/e2e/baseline/smoke/behavior/test_supersede.py |
Adds live same-key supersede coverage. |
tests/e2e/baseline/smoke/behavior/test_reconnect.py |
Adds normal-close reconnect coverage. |
tests/e2e/baseline/expected-cells.json |
Updates the smoke-test manifest. |
tests/conftest.py |
Shares a terminal-supersede payload. |
src/band/testing/transport.py |
Adds normal-close transport helpers. |
src/band/testing/phoenix_server.py |
Adds a stalled-peer helper. |
src/band/testing/__init__.py |
Exports the close helper. |
src/band/runtime/runtime.py |
Clarifies control-delivery docstrings. |
src/band/runtime/execution.py |
Clarifies interruption docstrings. |
src/band/runtime/control.py |
Documents serialized control delivery. |
src/band/platform/link.py |
Processes controls through ordered tasks. |
src/band/client/streaming/client.py |
Adds reconnect policy and terminal shutdown handling. |
pyproject.toml |
Adds Hypothesis to development extras. |
Review effort: Balanced
AlexanderZ-Band
force-pushed
the
feat/fix-agent-stays-offline-forever-after-websocket-cl-INT-1808
branch
from
October 20, 2026 06:07
26567c6 to
cafd66a
Compare
Base automatically changed from
feat/fix-agent-stays-offline-forever-after-websocket-cl-INT-1808
to
main
October 20, 2026 08:11
AlexanderZ-Band
marked this pull request as draft
October 20, 2026 12:07
This branch has not been deployed
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.
Why
tests/websocket/test_client.pyholds 53 module-level test functions across six unrelated concerns, which breaks the repo's one-class-per-file, one-concern-per-class test rule. #869 touches the file, and the rule asks that a touched file be split. Doing it there would bury the reconnect fix under a 1300-line move.What
tests/websocket/client/package:kit.pyfor the shared payloads,dispatch, upgrade-error builders and the in-process Phoenix peer;selfand two helper renames. The 58 collected test ids are identical before and after.Scope
Tests and comments only, with no behavior change. Stacked on #869.
Linear: INT-1808
🤖 Generated with Claude Code
https://claude.ai/code/session_01PBt2TLCc2YD1AXxmmo61Hx