Skip to content

fix(qqbot): add missing is_reconnect parameter to connect() method - #54547

Closed
shibuzhan wants to merge 1 commit into
NousResearch:mainfrom
shibuzhan:fix/qqbot-connect-is_reconnect
Closed

fix(qqbot): add missing is_reconnect parameter to connect() method#54547
shibuzhan wants to merge 1 commit into
NousResearch:mainfrom
shibuzhan:fix/qqbot-connect-is_reconnect

Conversation

@shibuzhan

Copy link
Copy Markdown

Problem

The base class BasePlatformAdapter defines connect() with the signature:

async def connect(self, *, is_reconnect: bool = False) -> bool:

But the QQ adapter's connect() was missing the is_reconnect parameter:

async def connect(self) -> bool:

This causes the gateway's reconnect watcher to fail when re-establishing the QQ connection after an outage, because it passes is_reconnect=True to all adapters.

Fix

Added the missing *, is_reconnect: bool = False parameter to match the base class interface. The QQ adapter uses a WebSocket connection and has no server-side message queue to preserve, so the flag is accepted and ignored (per the base class docstring: "Adapters with no such queue may ignore the flag").

Related

This was discovered when Hermes updated the gateway with reconnect logic but the QQ adapter hadn't been updated to match.

The base class (BasePlatformAdapter) defines connect() with
'*, is_reconnect: bool = False' signature, but the QQ adapter
was missing this parameter. This caused the gateway's reconnect
watcher to fail when re-establishing the QQ connection after
an outage, since it passes is_reconnect=True.

QQ adapter uses WebSocket so there is no server-side message
queue to preserve; the flag is accepted and ignored.
@alt-glitch alt-glitch added type/bug Something isn't working 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 P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jun 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #52922 — byte-identical one-line fix adding the keyword-only is_reconnect: bool = False parameter to QQAdapter.connect() to match the BasePlatformAdapter.connect() contract. #52922 (by @iborazzi) is the earliest open canonical fix for #52914 and is still open; this is part of a saturated duplicate cluster (#53546, #53948, #54165, #54037 all duplicate #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: LGTM

Simple fix that adds the missing is_reconnect parameter to the connect() method.

Key observations:

  • Backward compatible: The parameter has a default value of False, so existing callers are unaffected.
  • Minimal change: Only 1 line changed, well-scoped to the specific issue.
  • No security concerns: The parameter is used for connection lifecycle management.

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused QQBot reconnect fix.

Automated hermes-sweeper review found that current main already implements this exact behavior:

  • gateway/platforms/qqbot/adapter.py:281 accepts *, is_reconnect: bool = False and documents that QQBot accepts it for interface conformance.
  • Commit 276542c729c10ff9d093760897f4c2d1256a79ce added this QQBot compatibility fix.
  • tests/gateway/test_qqbot.py:193 covers both connect() and connect(is_reconnect=True).

Closing as implemented on main; this also aligns with the earlier duplicate-cluster discussion.

@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