Skip to content

fix(qqbot): add is_reconnect param to QQAdapter.connect for gateway reconnect compat - #52966

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

luxuguang-leo wants to merge 1 commit into
NousResearch:mainfrom
luxuguang-leo:fix/qqbot-is-reconnect

Conversation

@luxuguang-leo

Copy link
Copy Markdown
Contributor

Problem

The gateway reconnect watcher calls adapter.connect(is_reconnect=True) on every platform adapter during reconnection. QQAdapter's connect() did not accept the is_reconnect keyword argument, causing:

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

This leads to an infinite retry loop — every reconnect attempt fails with the same TypeError, the watcher backs off and retries, ad infinitum.

Root Cause

The base adapter's connect() abstract method was updated to include the is_reconnect parameter (commit 43b8ba4181 or earlier). All other platform adapters (Telegram, Discord, Feishu, WeChat, Signal, etc.) were updated to match the new signature. QQAdapter was missed.

Fix

Add *, is_reconnect: bool = False to QQAdapter.connect()'s signature. QQBot has no server-side update queue (unlike Telegram's Bot API), so the flag is accepted for interface conformance only — it is not used within the method.

Testing

  • New regression test: test_connect_accepts_is_reconnect_param verifies both adapter.connect() and adapter.connect(is_reconnect=True) complete without raising TypeError.
  • All 116 existing QQBot tests continue to pass (the 46 pre-existing failures on this machine are all due to missing aiohttp/httpx packages, none related to this change).

References

…econnect compat

The base adapter's  signature was updated to include
, which the reconnect watcher passes as
 during reconnection. All other platform adapters were
updated, but QQAdapter was missed, causing:

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

This leads to an infinite retry loop since every reconnect attempt fails
immediately with the same TypeError.

Fix: add  to QQAdapter.connect()'s signature.
QQBot has no server-side update queue, so the flag is accepted only for
interface conformance.

Test: new test_connect_accepts_is_reconnect_param verifies both
adapter.connect() and adapter.connect(is_reconnect=True) succeed without
raising.
@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 duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists labels Jun 26, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #52922 — same one-line fix (add keyword-only is_reconnect param to QQAdapter.connect()) for the same issue #52914. #52922 is the earlier open PR; maintainer should pick one. Both fix the TypeError -> infinite 60s reconnect backoff regression.

@Uesrmnae

Copy link
Copy Markdown

I independently encountered and fixed this same bug. This PR's approach is correct — it matches the *, is_reconnect: bool = False signature used by all 9 other platform adapters (base.py, weixin.py, signal.py, yuanbao.py, etc.).

The fix is clean, focused, and includes a test (test_connect_accepts_is_reconnect_param) that validates both connect() with default and connect(is_reconnect=True).

One note: PR #52922 has encoding corruption throughout its diff (UTF-8 BOM injection, em-dash U+2014 garbled to mojibake, Chinese text corrupted). That diff would need significant cleanup before it could merge. This PR (#52966) is the clean fix.

From an independent reviewer who arrived at the exact same fix.

@Ayuilos

Ayuilos commented Jul 1, 2026

Copy link
Copy Markdown

When will this fix be merged?

@alt-glitch alt-glitch removed the duplicate This issue or pull request already exists label Jul 1, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Re-triage correction (follow-up to the earlier "Duplicate of #52922" note above): that label is now stale. #52922 has since been CLOSED (not merged), and a resubmission is not a duplicate of a closed item.

This PR (created 2026-06-26) is now the earliest open fix PR for #52914, and the contract gap is still live on maingateway/platforms/qqbot/adapter.py still carries the bare def connect(self) -> bool:. This PR's diff is also the clean one-line signature fix without the UTF-8-BOM/mojibake encoding churn that affected #52922.

Accordingly the duplicate label has been removed and this is treated as the canonical open fix. related_to: #52914 (issue), #52922 (closed prior canonical), and open siblings #54029 / #54037. Priority held at P2 (whole-QQBot-gateway reconnect regression; lower-traffic gateway).

@mapleafgo

Copy link
Copy Markdown

+1 — independently hit the same reconnect loop on a headless QQ Bot setup. Applied the exact same one-line fix locally and the full handshake (token → WebSocket → ready) completes cleanly. Would be great to see this merged. Thanks @luxuguang-leo.

@luxuguang-leo

Copy link
Copy Markdown
Contributor Author

@teknium1 mind taking a look at this one? Been stuck on action_required for a while — fix independently verified, clean diff with regression test. Also have #52908 and #52973 in the same boat. Happy to rebase or close if salvage is easier. Thanks!

@kingsznhone

kingsznhone commented Jul 7, 2026

Copy link
Copy Markdown

I just ran into this exact bug. QQ Bot is completely bricked out of the box , the gateway starts, shows active, but QQ never connects. First-time users waste hours debugging because systemctl status lies and the real error is buried in a log file.

This isn't cosmetic. QQ Bot is one of the most requested gateway platforms for the Asian market, and it is entirely non-functional on current main. The fix is a single keyword parameter one character class of a diff. It's been independently verified by hundreds and thounsands users across the world against the live QQ API.

And yet, 11 days later, CI never triggered, zero reviews, stuck at action_required. #52922 closed. #54029, #54037, #56550, and others. all the same fix, all ignored. That's not triage. it's neglect of a platform integration that ships as part of the core product.

A one-line fix for a flagship feature, confirmed working by multiple people, sits rotting while everyone files duplicate PRs hoping one gets noticed. Please merge it or mark it salvage so someone can take over.

@luxuguang-leo

luxuguang-leo commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the callout @kingsznhone — you're right, the QQ Bot situation has been dragging too long.

@teknium1 @tonydwb could either of you take a look at this one when you get a chance? It's a minimal one-line interface fix (*, is_reconnect: bool = False) independently verified by several users now, and the same pattern is already used by every other platform adapter in the codebase. The CI check is stuck at action_required awaiting maintainer approval for an external fork.

Also have two related clean-up PRs if you're doing a pass:

Happy to rebase or adjust anything. Thanks!

@hsdwww

hsdwww commented Jul 8, 2026

Copy link
Copy Markdown

@teknium1 @tonybwb

This is a one-line fix that has been independently verified by multiple users and reviewed as LGTM by the automated reviewer. Every other platform adapter (Telegram, Discord, WeChat, Signal, etc.) already has this signature — QQBot is the only one still broken.

The PR has been sitting for 12 days with CI stuck at action_required awaiting maintainer approval for an external fork. Could one of you take a look and merge? Thanks!

@suntao12138

Copy link
Copy Markdown

This PR is mergeable (no conflicts) and the fix is confirmed working locally. Any chance this could get reviewed and merged? 🙏

@teknium1

Copy link
Copy Markdown
Collaborator

Merged via PR #61767. Your QQ Bot reconnect fix was cherry-picked onto current main, the same contract bug was fixed in WeCom Callback, and repository-wide regression coverage was added. Your authorship is preserved in git history. Thank you.

@teknium1 teknium1 closed this Jul 10, 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: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.

[Bug]: fix(qqbot): QQBot adapter.connect() missing is_reconnect parameter causes infinite retry loop

10 participants