Skip to content

fix: accept qqbot reconnect keyword - #60922

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

fix: accept qqbot reconnect keyword#60922
binbin2333 wants to merge 1 commit into
NousResearch:mainfrom
binbin2333:fix/qqbot-is-reconnect

Conversation

@binbin2333

Copy link
Copy Markdown

Summary

  • accept the gateway reconnect keyword in QQAdapter.connect
  • document that QQBot currently uses the same bootstrap path for initial connects and reconnects
  • add a regression test covering connect(is_reconnect=True) so runner-driven reconnects no longer crash on signature mismatch

Testing

  • /mydata/hermes/hermes-agent/venv/bin/python -m py_compile gateway/platforms/qqbot/adapter.py tests/gateway/test_qqbot.py
  • /mydata/hermes/hermes-agent/venv/bin/python -m pytest tests/gateway/test_qqbot.py -q -o addopts=''

@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 Jul 8, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.
Duplicate of #52966 (earliest still-open canonical fix for #52914). Same one-line signature fix: adding the keyword-only is_reconnect: bool = False to QQAdapter.connect() so the gateway reconnect watcher no longer raises TypeError. Note the earlier #52922 is closed/unmerged, so #52966 is the canonical anchor. Saturated cluster also includes #53948, #54029, #59353, #58537.

@kyssta-exe

Copy link
Copy Markdown
Contributor

Closing as duplicate — qqbot reconnect handling is already covered by earlier merged PRs. The duplicate label was correctly applied.

@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: Comment (21 additions — accept qqbot reconnect keyword)

Scope

Fix to accept qqbot reconnect keyword.

Observations

  • Small, targeted fix.
  • Well-scoped.

Recommendation

Looks reasonable for a small fix. Human reviewer familiar with the qqbot platform should confirm.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused QQBot regression fix.

Automated hermes-sweeper review found that this exact behavior is already on current main via merged PR #61767 (which cherry-picked the canonical #52966 work):

  • gateway/platforms/qqbot/adapter.py:281 now accepts *, is_reconnect: bool = False.
  • tests/gateway/test_qqbot.py:193 covers connect(is_reconnect=True).
  • tests/gateway/test_adapter_connect_is_reconnect_contract.py:126 adds repository-wide coverage against this adapter-contract drift.

The earlier duplicate discussion correctly pointed to #52966; that work is now preserved in commit 276542c729c10ff9d093760897f4c2d1256a79ce. Closing as implemented on main.

@teknium1 teknium1 closed this Jul 10, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label 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 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.

5 participants