fix(qqbot): prevent CPU-spinning tight loop after WebSocket reconnect failure (#17703)

[k176060444-lgtm]

Summary

Fixes #17703 — QQBot adapter enters a CPU-spinning tight loop after a failed reconnect, starving the asyncio event loop and making the gateway unresponsive.

Root Cause

When a QQ Bot WebSocket disconnects and the first reconnect attempt fails (e.g. DNS resolution during a brief network outage), _reconnect() returns False without clearing self._ws. The stale closed WebSocket reference causes _read_events() to exit its while loop immediately (because self._ws.closed is True), returning normally without raising. The outer _listen_loop treats this as a clean read-loop exit, resets backoff_idx = 0, and re-enters _read_events() instantly — creating a tight loop with no await points that starves the event loop and spins at 100% CPU.

03:31  WebSocket closed → _reconnect(0) → DNS fail → return False
       _ws is still non-None but closed
03:31  _read_events() → while condition False → returns normally
       backoff_idx = 0 (reset!)
       _read_events() → returns normally → backoff_idx = 0
       ...infinite tight loop, no sleep, no logging, no reconnect...
       CPU 100%, gateway unresponsive, eventually force-killed

Fix

Three independent guards, any one of which alone prevents the tight loop:

Guard 1 — _reconnect() root-cause cleanup (PR #20994 approach):

except Exception as exc:
    ...
    self._ws = None  # clear stale reference
    return False

Guard 2 — _read_events() entry check (PR #29057 approach):

if not self._ws or self._ws.closed:  # was: if not self._ws
    raise RuntimeError("WebSocket not connected")

Guard 3 — _read_events() post-loop fallback (PR #31333 approach):

# After the while loop exits without raising:
if self._running:
    raise RuntimeError("WebSocket closed")

Defense-in-depth: each guard is independent. If any future change accidentally removes one, the other two still prevent the tight loop.

Verification

Tested on a live Windows gateway with a manual router reboot:

Before fix: Gateway entered tight CPU spin after first reconnect failure, had to be force-killed.

After fix:

08:26:48  WebSocket error: WebSocket closed
08:26:48  Reconnecting in 2s (attempt 1)...
08:27:20  Reconnect failed: DNS failure
08:27:20  WebSocket error: WebSocket not connected  ← Guard 1 fired
08:27:20  Reconnecting in 5s (attempt 2)...         ← backoff continued!
08:27:46  Reconnect failed: DNS failure
08:27:46  Reconnecting in 10s (attempt 3)...
08:28:07  Reconnect failed: DNS failure
08:28:07  Reconnecting in 30s (attempt 4)...
08:28:38  ✅ Reconnected, session resumed            ← auto-recovered!

Gateway recovered automatically after ~50 seconds. No manual intervention needed.

Related PRs

This PR consolidates the approaches from five competing PRs (#29057, #27821, #20994, #31333, #30285) into a single defense-in-depth fix.

[liuhao1024]

Verified: this correctly fixes the CPU-spinning bug in _listen_loop.

Root cause confirmed — when _reconnect() fails on current main, self._ws remains as a closed/stale socket. _read_events() checks if not self._ws: (False — the object exists), enters the while loop, sees self._ws.closed is True, skips the body, and returns normally. Back in _listen_loop, backoff_idx = 0 resets and the cycle repeats immediately with no await suspension — 100% CPU.

Three-layer defense is correct:

1. self._ws = None in _reconnect() on failure — prevents stale reference from surviving into the next call.
2. if not self._ws or self._ws.closed: entry guard — raises immediately instead of entering a no-op loop.
3. Post-loop if self._running: raise RuntimeError("WebSocket closed") — catches the race where ws.closed flips between the entry guard and the loop condition.

I verified that _listen_loop (line 496–497) always resets backoff_idx = 0 after a normal return from _read_events(), confirming the tight-loop path. The post-loop fallback correctly checks self._running (not self._ws) because the adapter is still logically alive — only the transport died.

Docstring and inline comments clearly explain the invariant. Tests are focused. LGTM.

[k176060444-lgtm]

Local pytest results (Windows, Python 3.11.15)

Replaces CI which is not triggered for fork PRs.

============================= test session starts =============================
platform win32 -- Python 3.11.15, pytest-9.0.2, pluggy-1.6.0
rootdir: C:\Users\KK\hermes-agent
plugins: anyio-4.13.0, asyncio-1.3.0

tests/gateway/test_qqbot.py::TestCPUSpinningRegression::test_read_events_raises_when_ws_closed PASSED [ 20%]
tests/gateway/test_qqbot.py::TestCPUSpinningRegression::test_read_events_raises_when_ws_none PASSED [ 40%]
tests/gateway/test_qqbot.py::TestCPUSpinningRegression::test_read_events_closed_msg_raises PASSED [ 60%]
tests/gateway/test_qqbot.py::TestCPUSpinningRegression::test_reconnect_failure_clears_ws_and_session PASSED [ 80%]
tests/gateway/test_qqbot.py::TestCPUSpinningRegression::test_reconnect_failure_no_session_leak PASSED [100%]

====================== 5 passed, 159 deselected in 4.72s ======================

Test coverage

Test	Guard
test_read_events_raises_when_ws_closed	Entry: reject stale closed socket
test_read_events_raises_when_ws_none	Entry: reject None socket
test_read_events_closed_msg_raises	Loop: CLOSED message raises immediately
test_reconnect_failure_clears_ws_and_session	Cleanup: both _ws and _session cleared
test_reconnect_failure_no_session_leak	Edge: no crash if _session already None

Note: test_read_events_post_loop_guard was renamed to test_read_events_closed_msg_raises to accurately reflect it tests the CLOSED message branch inside the loop, not the post-loop fallback (which is nearly impossible to trigger in isolation).

[k176060444-lgtm]

Updated pytest results — 6 tests now cover all 3 guards

============================= test session starts =============================
platform win32 -- Python 3.11.15, pytest-9.0.2, pluggy-1.6.0
rootdir: C:\Users\KK\hermes-agent
plugins: anyio-4.13.0, asyncio-1.3.0

tests/gateway/test_qqbot.py::TestCPUSpinningRegression::test_read_events_raises_when_ws_closed PASSED [ 16%]
tests/gateway/test_qqbot.py::TestCPUSpinningRegression::test_read_events_raises_when_ws_none PASSED [ 33%]
tests/gateway/test_qqbot.py::TestCPUSpinningRegression::test_read_events_closed_msg_raises PASSED [ 50%]
tests/gateway/test_qqbot.py::TestCPUSpinningRegression::test_read_events_post_loop_guard PASSED [ 66%]
tests/gateway/test_qqbot.py::TestCPUSpinningRegression::test_reconnect_failure_clears_ws_and_session PASSED [ 83%]
tests/gateway/test_qqbot.py::TestCPUSpinningRegression::test_reconnect_failure_no_session_leak PASSED [100%]

====================== 6 passed, 159 deselected in 4.86s ======================

Test coverage (updated)

Test	Guard
test_read_events_raises_when_ws_closed	Entry: reject stale closed socket
test_read_events_raises_when_ws_none	Entry: reject None socket
test_read_events_closed_msg_raises	Loop: CLOSED message raises immediately
test_read_events_post_loop_guard	Post-loop: ws.closed flips False→True, loop never executes, fallback raises
test_reconnect_failure_clears_ws_and_session	Cleanup: both _ws and _session cleared
test_reconnect_failure_no_session_leak	Edge: no crash if _session already None

The new test_read_events_post_loop_guard uses a mock where ws.closed returns False on the first check (entry guard) and True on the second check (while condition), so the while-loop never executes and the post-loop fallback is exercised.

[k176060444-lgtm]

Superseded by #41773.

The primary closed-WebSocket CPU busy-loop guard has already been merged upstream. The remaining reconnect-failure resource cleanup has been extracted into a focused PR:

• fix(qqbot): clean up resources after reconnect failure #41773 — fix(qqbot): clean up resources after reconnect failure

Closing this PR to avoid duplicate review.