Skip to content

fix(qqbot): fix media send-back, interaction events, and WebSocket resilience - #27944

Closed
WideLee wants to merge 5 commits into
NousResearch:mainfrom
WideLee:feat/walli_20250518_bugfix
Closed

fix(qqbot): fix media send-back, interaction events, and WebSocket resilience#27944
WideLee wants to merge 5 commits into
NousResearch:mainfrom
WideLee:feat/walli_20250518_bugfix

Conversation

@WideLee

@WideLee WideLee commented May 18, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes multiple QQBot platform issues discovered during integration testing: video/file media cannot be sent back to users, interaction button events (approvals) are silently dropped, WebSocket reconnection doesn't handle server-initiated disconnects properly, and cached files have unreadable CDN hash names instead of their original filenames.

Root causes identified by diffing against the reference qqbot-agent-sdk implementation.

Related Issue

Fixes internal QQBot platform test failures (video send-back, file send-back, quoted video/file, approval button clicks).

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

1. Add INTERACTION intent (gateway/platforms/qqbot/adapter.py)

  • _send_identify() intents was missing 1 << 26 (INTERACTION bit), causing the QQ WebSocket gateway to never dispatch INTERACTION_CREATE events — approval button clicks were silently lost.

2. Expose video/file cached paths to LLM (gateway/platforms/qqbot/adapter.py)

  • _process_attachments() now formats non-image/voice attachments as [video: name (path)] or [file: name (path)] instead of the opaque [Attachment: name], so the LLM can reference local paths when the user asks to send media back.

3. Handle WebSocket op 7 and op 9 (gateway/platforms/qqbot/adapter.py)

  • op 7 (Server Reconnect): close WS gracefully to trigger the reconnect loop with session Resume — previously ignored, causing ~40s message loss during QQ gateway maintenance.
  • op 9 (Invalid Session): check payload.d to decide whether to Resume or re-Identify — previously ignored entirely.

4. Fix close code classification (gateway/platforms/qqbot/adapter.py)

  • Remove 4009 from session-clearing set (connection timeout is resumable per QQ protocol).
  • Expand fatal codes to include 4001, 4002, 4010-4014 (unrecoverable errors that should not retry).

5. Use original attachment filename for cached files (gateway/platforms/qqbot/adapter.py)

  • _download_and_cache() now accepts original_name parameter, preferring the attachment metadata filename over the CDN URL path basename (which is a meaningless hash like qqdownload_...oadftnv5).

6. Fix SILK magic byte detection (gateway/platforms/qqbot/adapter.py)

  • _guess_ext_from_data: data[:5] → data[:6] for the 6-byte b"#!SILK" literal.
  • _looks_like_silk: data[:4] → data[:6] (same issue).

7. Tests (tests/gateway/test_qqbot.py)

  • TestIdentifyIntents — verifies all 4 intent bits including INTERACTION.
  • TestProcessAttachmentsPathExposure — video/file path in descriptions, quoted message paths, download failure handling.
  • TestOp7ServerReconnect — op 7 triggers WS close, preserves session.
  • TestOp9InvalidSession — resumable vs non-resumable session handling.
  • TestCloseCodeClassification — 4009 not in clear set, fatal codes complete.

How to Test

  1. Start gateway with QQBot platform configured
  2. Send a video to the bot → ask it to send it back → should receive the video file (not just text)
  3. Send a file to the bot → ask it to send it back → should receive the file with correct filename
  4. Quote a video/file message → bot should acknowledge the content
  5. Click an approval button → should resolve the pending approval (check logs for INTERACTION_CREATE)
  6. Run pytest tests/gateway/test_qqbot.py -q → all 158 tests pass

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 (Apple Silicon)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

Test results:

======================= 158 passed, 4 warnings in 3.32s ========================

WideLee added 4 commits May 18, 2026 19:22
@WideLee
fix(qqbot): add INTERACTION intent and expose video/file cached paths
5ab33ce 
1. Add INTERACTION intent bit (1<<26) to _send_identify, fixing approval
   button clicks not being received (INTERACTION_CREATE events were never
   dispatched by the gateway)
2. Include local cached path in video/file attachment descriptions so the
   LLM can reference files for re-sending to users
3. Add unit tests (TestIdentifyIntents, TestProcessAttachmentsPathExposure)
@WideLee
fix(qqbot): handle op 7/9 and expand fatal close code set
ab584de 
1. Handle op 7 (Server Reconnect): close WS to trigger reconnect loop
   while preserving session for Resume
2. Handle op 9 (Invalid Session): check d value to determine if session
   is resumable; clear session only when not resumable
3. Remove 4009 from session-clearing set (connection timeout is resumable)
4. Expand fatal close codes: 4001/4002/4010-4014 now stop reconnect
   immediately instead of retrying uselessly
5. Add unit tests
@WideLee
fix(qqbot): use original attachment filename for cached files
351b161 
Add original_name parameter to _download_and_cache, preferring the
attachment metadata filename over the CDN URL path basename. Previously
files were cached with meaningless QQ CDN hash names (e.g.
qqdownload_...oadftnv5), causing ugly filenames when sent back to users.

Aligns with qqbot-agent-sdk's AttachmentDownloader.download_document.
@WideLee
fix(qqbot): fix SILK magic byte detection slice length
7c5a424 
_guess_ext_from_data: data[:5] == b"#!SILK" -> data[:6] (6-byte string)
_looks_like_silk: data[:4] == b"#!SILK" -> data[:6]

The previous slices were too short to ever match the 6-byte "#!SILK"
literal, relying entirely on the "#!SILK_V3" (9-byte) and 0x02! (2-byte)
fallback paths for SILK format detection.
@WideLee WideLee changed the title Feat/walli 20250518 bugfix fix(qqbot): fix media send-back, interaction events, and WebSocket resilience May 18, 2026
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/qqbot QQ Bot adapter comp/gateway Gateway runner, session dispatch, delivery labels May 18, 2026
@BoardJames-Bot

BoardJames-Bot commented May 18, 2026

Copy link
Copy Markdown

BoardJames triage: this looks shared/systemic rather than branch-local. The PR-specific checks (lint/nix/e2e/builds/attribution/history) are green where completed; the remaining blocker is the main Tests / test job, which is currently failing/timing out across unrelated PRs and on main itself (latest main run hit the same aux/session_search + kanban dashboard + compression/Anthropic test drift). I pushed the missing aux/session_search default fix onto the existing systemic fix PR #27931 and validated the affected files locally (125 passed). Next action is maintainer review/workflow approval/merge of #27931, then rerun this PR's Tests / test; no branch-local author action is indicated from the logs I can see.

@WideLee
fix(qqbot): dedicated WS thread + heartbeat restart + media send support
310cd3e 
Critical threading/concurrency fixes for the QQBot adapter, plus media
delivery support for the send_message tool.

## WebSocket dedicated thread

Move the QQBot WebSocket lifecycle (listen loop, heartbeat, reconnect)
to a dedicated daemon thread with its own asyncio event loop. This
prevents QQBot's reconnect backoff and heartbeat from blocking the
main gateway event loop shared with Discord, Feishu, Telegram, etc.

Same pattern as qqbot-agent-sdk's QQWebSocket and the Feishu adapter's
_run_official_feishu_ws_client.

Inbound message callbacks are dispatched back to the main loop via
asyncio.run_coroutine_threadsafe().

## Heartbeat restart on Hello (op 10)

Add _restart_heartbeat() — cancels the current heartbeat task and
creates a fresh one every time a Hello is received (both initial
connect and reconnect). This ensures the new heartbeat_interval takes
effect immediately instead of waiting for a stale asyncio.sleep() to
expire, which could delay the first heartbeat by up to 41s and trigger
a 4009 timeout from the QQ gateway.

## Thread-safe token management

- Replace asyncio.Lock with threading.Lock for _token_lock
- Add _ensure_token_sync() using httpx.Client (sync) for WS thread
- Add _get_gateway_url_sync() for WS thread
- _send_identify / _send_resume / _reconnect use sync token methods
- async _ensure_token() delegates to run_in_executor(sync_version)
  to avoid holding a threading.Lock while awaiting I/O

## QQBot media delivery via send_message tool

- Add _send_qqbot_media() — routes media through the running gateway
  adapter's upload pipeline (send_image_file/send_video/send_voice/
  send_document)
- Add QQBot media early-return block before the "Non-media platforms"
  error check
- Update error messages to include qqbot in supported platforms list

## Other fixes

- disconnect() no longer awaits tasks from the WS thread's loop
  (would raise "cannot await on a future created in a different loop")
- _ws_thread_main only awaits _listen_task; heartbeat is cancelled in
  finally (handles dynamic _restart_heartbeat replacements correctly)
- _dispatch_to_main_loop adds done_callback to log exceptions from
  dispatched coroutines (prevents silent exception swallowing)

Tests: 273 passed (158 qqbot + 115 send_message)
@alt-glitch alt-glitch mentioned this pull request May 22, 2026
Closed
@teknium1 teknium1 mentioned this pull request May 23, 2026
Merged
@teknium1

teknium1 commented May 23, 2026

Copy link
Copy Markdown
Contributor

Salvaged 4 of your 5 commits onto current main via #30843 — rebase-merged with your authorship preserved (walli@tencent.com). Landed:

  • bbd77d1 — INTERACTION intent + video/file path exposure
  • a54f5af — op 7/9 + fatal close codes (4001/4002/4010-4014, 4009 removed from session-clearing)
  • 0e7448d — original attachment filename
  • 60b0a0e — SILK magic byte slice fix

Deferred the 5th commit (310cd3e — dedicated WS thread + media send support) for a separate review with live gateway validation; that commit is a substantial concurrency rewrite (asyncio.Lock → threading.Lock, run_coroutine_threadsafe cross-loop dispatch) that supersedes #28167 and warrants live testing before merge.

Thanks for the fixes!

@teknium1 teknium1 closed this May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/qqbot QQ Bot adapter type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@WideLee @BoardJames-Bot @teknium1 @alt-glitch