Skip to content

fix(qqbot): accept is_reconnect kwarg in QQAdapter.connect - #54029

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

fix(qqbot): accept is_reconnect kwarg in QQAdapter.connect#54029
xflatcn wants to merge 1 commit into
NousResearch:mainfrom
xflatcn:fix/qqbot-connect-is-reconnect

Conversation

@xflatcn

@xflatcn xflatcn commented Jun 28, 2026

Copy link
Copy Markdown

Summary

The gateway reconnect loop calls adapter.connect(is_reconnect=True) (gateway/run.py forwards the flag on every retry via _connect_adapter), and the BasePlatformAdapter.connect contract declares is_reconnect as a keyword argument. Every other adapter (telegram, slack, signal, discord, …) accepts it — but QQAdapter.connect omitted it.

As a result, the first reconnect attempt after a dropped QQ connection raised:

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

and QQ never recovered — the gateway kept retrying with exponential backoff (60s → 120s → 240s → 300s) and the bot stayed offline. Observed in the wild on a live QQ bot deployment.

Fix

Add *, is_reconnect: bool = False to QQAdapter.connect to match the base-class contract. The QQ adapter establishes a fresh WebSocket session on both cold start and reconnect, so the flag is accepted for signature compatibility but not otherwise used (documented in the docstring).

Test

Adds test_connect_accepts_is_reconnect_kwarg which asserts the signature exposes is_reconnect and that connect(is_reconnect=True) does not raise TypeError on the reconnect path.

tests/gateway/test_qqbot.py::TestVoiceAttachmentSSRFProtection::test_connect_uses_redirect_guard_hook PASSED
tests/gateway/test_qqbot.py::TestVoiceAttachmentSSRFProtection::test_connect_accepts_is_reconnect_kwarg PASSED

The gateway reconnect loop calls adapter.connect(is_reconnect=True)
(gateway/run.py forwards the flag on every retry), and the
BasePlatformAdapter.connect contract declares is_reconnect as a keyword
argument. QQAdapter.connect omitted it, so every reconnect attempt raised
TypeError: QQAdapter.connect() got an unexpected keyword argument
'is_reconnect' and QQ never recovered after a dropped connection.

Add the keyword argument to match the contract. The QQ adapter opens a
fresh WebSocket session regardless, so the flag is accepted for signature
compatibility but not otherwise used. Adds a regression test asserting the
signature and that connect(is_reconnect=True) does not raise.
@tonydwb

tonydwb commented Jun 28, 2026

Copy link
Copy Markdown

Code Review Summary (Hermes Agent)

Verdict: Approved — Adds is_reconnect kwarg to QQAdapter.connect for BasePlatformAdapter signature compatibility. The gateway reconnect loop calls connect(is_reconnect=True) and the QQ adapter was missing this parameter, causing TypeError on reconnection.

Changes:

  • gateway/platforms/qqbot/adapter.py: Accepts is_reconnect kwarg with docstring explaining it's for signature compatibility
  • tests/gateway/test_qqbot.py: Regression test verifying the kwarg is accepted

Assessment:

  • Tiny, focused fix (2 files)
  • Proper docstring explaining the kwarg's purpose
  • Test covers the exact regression scenario
  • No security concerns

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/qqbot QQ Bot adapter P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages duplicate This issue or pull request already exists labels Jun 28, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #52922 — the earliest open PR making the identical one-line is_reconnect signature fix on QQAdapter.connect() for the same issue #52914. This is a saturated cluster (#53948/#53546/#52966/#53540 are already marked duplicate of #52922). Consolidating on #52922.

@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 (competing fix — see note)

This PR adds is_reconnect keyword argument to QQAdapter.connect() — the same fix as PR #54037. The implementations are functionally identical (both add the kwarg with the same docstring explanation). Note: PR #54037 was also submitted for this issue. The maintainer may want to merge one and close the other.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused QQ reconnect fix. This is already implemented on current main.

  • gateway/platforms/qqbot/adapter.py:281 already defines QQAdapter.connect(self, *, is_reconnect: bool = False).
  • tests/gateway/test_qqbot.py:193 already covers both the default connect call and connect(is_reconnect=True).
  • The implementation and regression test landed in 276542c729c10ff9d093760897f4c2d1256a79ce (fix(qqbot): add is_reconnect param to QQAdapter.connect for gateway reconnect compat).

Automated hermes-sweeper review.

@teknium1 teknium1 closed this Jul 15, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 15, 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 duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists platform/qqbot QQ Bot adapter sweeper:implemented-on-main Sweeper: behavior already present on current main 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.

4 participants