Skip to content

fix(qqbot): cap stale resume retries and fallback to identify - #22422

Open
chongru1988 wants to merge 1 commit into
NousResearch:mainfrom
chongru1988:fix/qqbot-resume-retries-fallback-identify-22384
Open

fix(qqbot): cap stale resume retries and fallback to identify#22422
chongru1988 wants to merge 1 commit into
NousResearch:mainfrom
chongru1988:fix/qqbot-resume-retries-fallback-identify-22384

Conversation

@chongru1988

Copy link
Copy Markdown

Summary

  • Limit consecutive QQ gateway Resume attempts to avoid endless reconnect loops on stale sessions.
  • Add a fallback path that clears stale session state and sends Identify after the resume cap is reached.
  • Reset resume-attempt state on recovery/error paths (READY, RESUMED, session-invalid, and resume-send failure).

Why

Some stale-session reconnect flows can repeatedly attempt Resume without making progress. This change makes recovery deterministic by forcing a clean Identify handshake after bounded retries.

Changes

  • gateway/platforms/qqbot/constants.py
    • add MAX_RESUME_ATTEMPTS
  • gateway/platforms/qqbot/adapter.py
    • track resume attempts
    • enforce cap in Hello auth mode selection
    • fallback to Identify when cap is reached
    • reset counter on success/error recovery paths
  • tests/gateway/test_qqbot.py
    • add regression tests for Hello auth mode selection

Test Plan

  • python -m pytest tests/gateway/test_qqbot.py -k "HelloAuthModeSelection or ReadyHandling" -q -n 4
  • Result: 4 passed

Linked Issue

Prevent endless resume loops on stale QQ sessions by limiting consecutive resume attempts and forcing Identify once the cap is reached.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/qqbot QQ Bot adapter labels May 9, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing fix — see #22384 which addresses the same QQBot Resume death-loop (#22179). Also related to #17990.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for targeting the stale QQ Resume path. The premise remains present on current main: the Hello handler still chooses Resume solely from _session_id / _last_seq (gateway/platforms/qqbot/adapter.py:830-834).

Problems

  • The cap cannot recover the PR's stated “immediately closes” case. The PR sends Resume for attempts 1–3 and selects Identify only on Hello 4, but current main exits after the third quick disconnect (gateway/platforms/qqbot/adapter.py:515-534). That prevents the fallback from running.
  • The new tests seed _resume_attempts at the cap, so they do not exercise the current listener's immediate-close / quick-disconnect path.

Suggested changes

  • Reconcile the Resume cap with the quick-disconnect cutoff so stale sessions reach Identify before listener termination, while retaining the current 4009-resumable behavior at gateway/platforms/qqbot/adapter.py:600-627.
  • Add an async regression covering consecutive immediate closes through the actual reconnect loop.

Automated hermes-sweeper review.

@@ -772,8 +776,21 @@ def _dispatch_payload(self, payload: Dict[str, Any]) -> None:
# Authenticate: send Resume if we have a session, else Identify.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fallback is only entered on the fourth Hello: the first three Hellos increment and send Resume. For the stated immediate-close failure, current main exits after its third quick disconnect (_listen_loop, gateway/platforms/qqbot/adapter.py:515-534), so this branch is never reached. Please coordinate the threshold with that guard and add a reconnect-loop regression.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/qqbot QQ Bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants