fix(aiocqhttp): evict stale reverse websocket connections

[jmt059]

Summary

• add a configurable inbound-frame timeout for aiocqhttp reverse WebSocket connections
• close stale connections so the OneBot client can reconnect after a half-open network failure
• replace older same-account connections without allowing their cleanup path to remove the new API mapping
• add focused tests for timeout, disabled timeout, close compatibility, replacement, and cleanup races

Motivation

After a transient network or NAT interruption, a reverse WebSocket can remain TCP-established while no longer delivering application frames. aiocqhttp 1.4.4 waits indefinitely in websocket.receive(), so the adapter cannot distinguish this state from a healthy idle connection. Restarting the adapter recovers the connection because it closes the stale socket, but there is no automatic recovery path.

The new ws_reverse_idle_timeout setting defaults to 60 seconds and can be set to 0 to disable the guard. In the validated NapCat setup, heartbeat and reconnect intervals are both 5 seconds, so the default requires roughly 12 consecutive missed heartbeat frames before eviction.

Verification

Validated against AstrBot master commit c6a14e0600485293bd88cf78c04ecec967b21b50:

python -m ruff check <changed files>                         passed
python -m ruff format --check <changed files>                passed
python -m compileall -q <changed files>                      passed
python -m pytest -q tests/unit/test_aiocqhttp_websocket_guard.py
8 passed

No new dependencies are introduced.

Checklist

• This is not a breaking API change.
• The change has focused automated tests.
• No new dependency is introduced.
• Logs contain the adapter label and no message contents or account identifiers.

Summary by Sourcery

Prevent stale aiocqhttp reverse WebSocket connections from blocking automatic client recovery.

New Features:

• Add a configurable idle timeout for inbound reverse WebSocket frames, with a default of 60 seconds and an option to disable it.

Bug Fixes:

• Automatically evict stale reverse WebSocket connections after prolonged inactivity so clients can reconnect after half-open network failures.
• Safely replace older same-account connections without allowing their cleanup to remove the active connection mapping.

Enhancements:

• Improve WebSocket closing compatibility across asynchronous and synchronous close implementations.

Tests:

• Add focused unit coverage for idle timeout behavior, disabled timeouts, close compatibility, connection replacement, and cleanup races.

[sourcery-ai]

Hey - I've found 1 issue

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="astrbot/core/platform/sources/aiocqhttp/guarded_cqhttp.py" line_range="147-157" />
<code_context>
+            self_id: OneBot self identifier supplied by the connection.
+            ws: Newly connected WebSocket.
+        """
+        previous = self._wsr_api_clients.get(self_id)
+        self._wsr_api_clients[self_id] = ws
+        if previous is None or previous is ws:
+            return
+
+        logger.warning(
+            "Replacing an existing aiocqhttp reverse WebSocket connection for "
+            "adapter %s.",
+            self.connection_label,
+        )
+        await self._close_ws(previous, code=1000, reason="Replaced by new connection")
+
+    def _remove_api_client(self, self_id: str, ws: Any) -> None:
</code_context>
<issue_to_address>
**issue (bug_risk):** `_register_api_client` installs the new WebSocket in `_wsr_api_clients` and then awaits the old socket's close operation before the handler enters its `try/finally`. If the handler is cancelled during that await, the new mapping remains permanently registered even though its handler has exited, so later API calls target a stale connection.

**Triggers:** When a same-account connection replaces an existing connection and the handler is cancelled while the old socket is being closed, such as during shutdown.

**Suggested fix:** Put registration and replacement inside the handler's cleanup scope, or roll back the mapping when cancellation interrupts `_register_api_client`.

```suggestion
        self_id = ws.headers["X-Self-ID"]
        try:
            await self._register_api_client(self_id, ws)
            while True:
                connected, payload = await self._receive_payload(ws)
                if not connected:
                    return
                if payload is not None:
                    ResultStore.add(payload)
        finally:
            self._remove_api_client(self_id, ws)
```
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 1 finding to address first, and if the timeout or replacement logic is wrong, active reverse-WebSocket connections could be closed and events or API responses could be disrupted until the clients reconnect. Reverting prevents further evictions, but it cannot restore connections that were already terminated or any transient work lost during those closures.

Blocking findings: astrbot/core/platform/sources/aiocqhttp/guarded_cqhttp.py:157

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): _register_api_client installs the new WebSocket in _wsr_api_clients and then awaits the old socket's close operation before the handler enters its try/finally. If the handler is cancelled during that await, the new mapping remains permanently registered even though its handler has exited, so later API calls target a stale connection.

Triggers: When a same-account connection replaces an existing connection and the handler is cancelled while the old socket is being closed, such as during shutdown.

Suggested fix: Put registration and replacement inside the handler's cleanup scope, or roll back the mapping when cancellation interrupts _register_api_client.

Suggested change

	self_id = ws.headers["X-Self-ID"]
	await self._register_api_client(self_id, ws)
	try:
	while True:
	connected, payload = await self._receive_payload(ws)
	if not connected:
	return
	if payload is not None:
	ResultStore.add(payload)
	finally:
	self._remove_api_client(self_id, ws)
	self_id = ws.headers["X-Self-ID"]
	try:
	await self._register_api_client(self_id, ws)
	while True:
	connected, payload = await self._receive_payload(ws)
	if not connected:
	return
	if payload is not None:
	ResultStore.add(payload)
	finally:
	self._remove_api_client(self_id, ws)

[jmt059]

Closing for now. This transport-level change has not yet been validated in a production deployment. I will only reopen or submit a revised PR after addressing the known race and compatibility concerns and completing real-world validation.