Skip to content

fix(qqbot): accept gateway reconnect flag - #59353

Closed
lordlink01 wants to merge 1 commit into
NousResearch:mainfrom
lordlink01:fix/qqbot-connect-reconnect-flag
Closed

fix(qqbot): accept gateway reconnect flag#59353
lordlink01 wants to merge 1 commit into
NousResearch:mainfrom
lordlink01:fix/qqbot-connect-reconnect-flag

Conversation

@lordlink01

@lordlink01 lordlink01 commented Jul 6, 2026

Copy link
Copy Markdown

Summary

  • update QQBot adapter to accept the gateway's connect(is_reconnect=...) keyword argument
  • document that QQBot manages resume vs identify internally and can ignore the flag
  • add a regression test that calls QQAdapter.connect(is_reconnect=True)

Why

GatewayRunner now forwards is_reconnect through _connect_adapter_with_timeout() to every adapter. QQBot still exposed connect(self), so gateway startup/reconnect failed before it could open the QQ websocket:

QQAdapter.connect() got an unexpected keyword argument 'is_reconnect'

Test Plan

  • python3 -m pytest tests/gateway/test_qqbot.py::TestVoiceAttachmentSSRFProtection::test_connect_accepts_gateway_reconnect_flag -q -o 'addopts='
  • python3 -m pytest tests/gateway/test_qqbot.py -q -o 'addopts='

Match the BasePlatformAdapter.connect(is_reconnect=...) contract so QQBot can be started by the gateway reconnect path. Add a regression test that calls QQAdapter.connect(is_reconnect=True).
@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 sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages duplicate This issue or pull request already exists labels Jul 6, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #52966 — identical one-line fix adding the keyword-only is_reconnect: bool = False parameter to QQAdapter.connect() (same function, same regression test). #52966 is the earliest still-open canonical fix for issue #52914 (the prior anchor #52922 is closed). This is a saturated cluster of byte-equivalent fixes for the same v0.17.0 reconnect regression.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Changes

Updates QQAdapter.connect() to accept is_reconnect: bool = False keyword argument for compatibility with the gateway's common PlatformAdapter.connect() contract. QQBot manages resume vs. cold identify internally from stored session state. Also adds a dedicated regression test.

Quality

  • Clean contract alignment: matches the base class signature
  • Well-documented with docstring explaining how QQBot handles the flag internally
  • Regression test covers the exact call pattern gateway.run uses

Suggestions

  • None

Reviewed by Hermes Agent

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

COMMENT: fix(qqbot): accept gateway reconnect flag

Small fix (21 additions, 2 deletions). No security concerns.


Reviewed by Hermes Agent

@yangguo

yangguo commented Jul 9, 2026

Copy link
Copy Markdown

I hit the same regression on a live Hermes gateway with QQBot enabled after updating to current origin/main (88a58ff13). The gateway process stayed up, but QQBot stayed in retrying state with:

QQAdapter.connect() got an unexpected keyword argument 'is_reconnect'

The local gateway state showed Telegram/Discord connected while QQBot remained retrying with that error.

I independently tested the same minimal fix locally and this PR is the cleaner version of it: QQBot accepts the gateway reconnect kwarg without changing its QQ auth/WebSocket behavior.

Additional local validation I ran:

scripts/run_tests.sh tests/gateway/test_qqbot.py -q
# 165 passed

scripts/run_tests.sh tests/gateway/test_platform_reconnect.py tests/gateway/relay/test_relay_adapter.py -q
# 52 passed

.venv/bin/python -m ruff check gateway/platforms/qqbot/adapter.py tests/gateway/test_qqbot.py
# All checks passed

I opened a duplicate PR while debugging (#61356) and closed it in favor of this one.

@lordlink01

Copy link
Copy Markdown
Author

The official reconnect-contract fix has now landed via #61767, which includes the QQBot regression fix and repository-wide adapter contract coverage. Closing this duplicate now that upstream main contains the fix. Thank you.

@lordlink01 lordlink01 closed this Jul 10, 2026
@lordlink01
lordlink01 deleted the fix/qqbot-connect-reconnect-flag branch July 10, 2026 07:35
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 duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists platform/qqbot QQ Bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants