fix(qqbot): cap Resume retries and fall back to Identify on stale session - #22384
Closed
wesleysimplicio wants to merge 1 commit into
Closed
fix(qqbot): cap Resume retries and fall back to Identify on stale session#22384wesleysimplicio wants to merge 1 commit into
wesleysimplicio wants to merge 1 commit into
Conversation
…sion Problem ------- When the QQ gateway accepts a Resume frame but immediately closes the WebSocket (the typical signal that the session_id has expired server-side), the adapter previously kept reconnecting and retrying Resume forever with the same expired session_id. Bot stays effectively offline until the process is restarted. Root cause ---------- The op-10 Hello handler picks Resume whenever ``_session_id`` and ``_last_seq`` are non-None, with no notion of how many consecutive Resume attempts have not been confirmed by a RESUMED/READY dispatch. A server that accepts Resume then closes the socket leaves both fields populated, so every reconnect picks Resume again. Fix --- Track ``_resume_attempts`` on the adapter. Each op-10 Hello that picks Resume increments it; after ``MAX_RESUME_ATTEMPTS = 3`` consecutive unconfirmed attempts, discard ``_session_id``/``_last_seq`` and fall back to a fresh Identify. The counter resets on RESUMED dispatch and on successful READY (handled in ``_handle_ready``), so a healthy reconnect cycle is unaffected. Tests ----- ``tests/gateway/test_qqbot_resume_loop_fallback.py`` drives op-10 Hello through ``_dispatch_payload`` and asserts: * counter starts at 0 * Resume increments the counter without discarding the session * (cap+1)-th Hello discards ``_session_id``/``_last_seq`` * RESUMED and READY both reset the counter * absence of ``_session_id`` keeps the counter at 0 (Identify path) * after fallback, a fresh session restarts the cycle from 0 * a source-level guard keeps ``MAX_RESUME_ATTEMPTS`` configurable Verified failing on ``main`` (8/8) and passing on this branch (8/8). Full ``tests/gateway/`` suite still green (5050 passed). Pre-existing unrelated failures in ``tests/tools/test_file_read_guards``, ``tests/hermes_cli/test_gateway_wsl``, etc. reproduce on ``main`` without this change and are out of scope. Closes NousResearch#22179 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a QQ Bot gateway reconnection failure mode where an expired session_id could cause the adapter to endlessly retry Resume and never fall back to a fresh Identify, leaving the bot effectively offline until restart.
Changes:
- Add
MAX_RESUME_ATTEMPTS(default 3) to cap consecutive unconfirmed Resume attempts. - Track
_resume_attemptsinQQAdapterand discard stale session state after the cap, forcing Identify. - Add a regression test suite to validate the retry cap and counter reset behavior on
READY/RESUMED.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
gateway/platforms/qqbot/adapter.py |
Adds _resume_attempts tracking and Hello-time fallback from Resume to Identify after capped failures. |
gateway/platforms/qqbot/constants.py |
Introduces configurable MAX_RESUME_ATTEMPTS constant used by the adapter. |
tests/gateway/test_qqbot_resume_loop_fallback.py |
Adds regression tests covering the stale-session resume loop and fallback/reset conditions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
794
to
+798
| else: | ||
| if self._resume_attempts >= MAX_RESUME_ATTEMPTS: | ||
| logger.warning( | ||
| "[%s] Resume failed %d times — discarding stale session_id and re-identifying", | ||
| self._log_tag, |
Comment on lines
+17
to
+18
| import pytest | ||
|
|
Contributor
Author
|
Closing — PR has merge conflicts that can't be auto-resolved. The codebase has evolved past this fix. Re-opening with a fresh rebase welcome if the issue is still open. |
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.
What does this PR do?
When the QQ gateway accepts a Resume frame but immediately closes the WebSocket (the typical signal that the
session_idhas expired server-side), the adapter previously kept reconnecting and retrying Resume forever with the same expiredsession_id. The bot stays effectively offline until the process is restarted.Root cause
When the QQ gateway accepts a Resume frame but immediately closes the WebSocket (the typical signal that the
session_idhas expired server-side), the adapter previously kept reconnecting and retrying Resume forever with the same expiredsession_id. The bot stays effectively offline until the process is restarted.Fix
Track
_resume_attemptson the adapter. Each op-10 Hello that picks Resume increments it; afterMAX_RESUME_ATTEMPTS = 3consecutive unconfirmed attempts, discard_session_id/_last_seqand fall back to a fresh Identify. The counter resets on RESUMED dispatch and on successful READY (handled in_handle_ready), so a healthy reconnect cycle is unaffected.The cap is exposed as
MAX_RESUME_ATTEMPTSingateway/platforms/qqbot/constants.pyso future tuning is a one-line change.Why this shape
This shape mirrors #29640 so reviewers can quickly compare scope, root cause, fix, tests, and related context without having to decode a custom PR description.
Tests
Original body
Related PRs / issues
Closes #22179
Original body
Summary
When the QQ gateway accepts a Resume frame but immediately closes the WebSocket (the typical signal that the
session_idhas expired server-side), the adapter previously kept reconnecting and retrying Resume forever with the same expiredsession_id. The bot stays effectively offline until the process is restarted.What Changed
Fluxo
A mudança continua seguindo o fluxo original descrito na seção preservada abaixo, sem ampliar o escopo funcional deste PR.
Visão
A padronização melhora a revisão, reduz ruído e evita deriva de formatação entre PRs abertos.
Test Plan
Original body
What does this PR do?
Problem
When the QQ gateway accepts a Resume frame but immediately closes the WebSocket (the typical signal that the
session_idhas expired server-side), the adapter previously kept reconnecting and retrying Resume forever with the same expiredsession_id. The bot stays effectively offline until the process is restarted.Root cause
gateway/platforms/qqbot/adapter.pyop-10 Hello handler picks Resume whenever_session_idand_last_seqare non-None, with no notion of how many consecutive Resume attempts have not been confirmed by a RESUMED/READY dispatch. A server that accepts Resume then closes the socket leaves both fields populated, so every reconnect picks Resume again.Fix
Track
_resume_attemptson the adapter. Each op-10 Hello that picks Resume increments it; afterMAX_RESUME_ATTEMPTS = 3consecutive unconfirmed attempts, discard_session_id/_last_seqand fall back to a fresh Identify. The counter resets on RESUMED dispatch and on successful READY (handled in_handle_ready), so a healthy reconnect cycle is unaffected.The cap is exposed as
MAX_RESUME_ATTEMPTSingateway/platforms/qqbot/constants.pyso future tuning is a one-line change.Tests
tests/gateway/test_qqbot_resume_loop_fallback.pydrives op-10 Hello through_dispatch_payloadand asserts:_session_id/_last_seq_session_idkeeps the counter at 0 (Identify path)MAX_RESUME_ATTEMPTSconfigurableVerified failing on
main(8/8) and passing on this branch (8/8). Fulltests/gateway/suite still green (5050 passed). Pre-existing unrelated failures elsewhere in the suite (tests/tools/test_file_read_guards,tests/hermes_cli/test_gateway_wsl, etc.) reproduce onmainwithout this change and are out of scope.Closes #22179
Solution Sketch
Related Issue
Closes #22179
Type of Change
Changes Made
.github/PULL_REQUEST_TEMPLATE.mdHow to Test
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
Generated by Hermes Turbo
Generated by Hermes Turbo