Skip to content

fix(qqbot): track inbound dispatch tasks so received messages aren't dropped - #41298

Open
Vesna-9 wants to merge 1 commit into
NousResearch:mainfrom
Vesna-9:fix/qqbot-track-inbound-dispatch-tasks
Open

fix(qqbot): track inbound dispatch tasks so received messages aren't dropped#41298
Vesna-9 wants to merge 1 commit into
NousResearch:mainfrom
Vesna-9:fix/qqbot-track-inbound-dispatch-tasks

Conversation

@Vesna-9

@Vesna-9 Vesna-9 commented Jun 7, 2026

Copy link
Copy Markdown

What does this PR do?

The QQ Bot adapter spawned its inbound-message handler with a bare
asyncio.create_task(self._on_message(t, d)) in _dispatch_payload. The
asyncio event loop only keeps a weak reference to a task created that way, so
with no strong reference held anywhere the garbage collector is free to cancel
the handler mid-flight — and when it does, the received QQ message is silently
dropped with no error. Every other branch in _dispatch_payload (Resume,
Identify, interaction, server-reconnect close) already routes through the
adapter's _create_task helper; only the message branch did not.

This routes the message branch through _create_task and makes that helper
hold each task in the inherited self._background_tasks set until it finishes
(discarding it via a done-callback), exactly as BasePlatformAdapter already
does for its own spawned tasks. As a side effect it also fixes a test-safety
gap: _create_task was created specifically so _dispatch_payload can be
called synchronously outside asyncio.run() without raising "no running event
loop", but the message branch bypassed it — so a message op was the one payload
type that could not be dispatched the way the existing synchronous tests
dispatch every other op.

Related Issue

N/A

Type of Change

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

Changes Made

  • gateway/platforms/qqbot/adapter.py: route the inbound-message dispatch
    (C2C_MESSAGE_CREATE, GROUP_AT_MESSAGE_CREATE, DIRECT_MESSAGE_CREATE,
    GUILD_MESSAGE_CREATE, GUILD_AT_MESSAGE_CREATE) through self._create_task
    instead of a bare asyncio.create_task.
  • gateway/platforms/qqbot/adapter.py: change _create_task from a
    @staticmethod into an instance method that registers each spawned task in
    self._background_tasks and removes it again with an add_done_callback
    discard, so a still-running handler can't be garbage-collected. All call
    sites already invoked it as self._create_task(...), so they are unchanged.
  • tests/gateway/test_qqbot.py: add two tests in TestDispatchPayload
    test_message_dispatch_without_event_loop_does_not_raise (a message op
    dispatched synchronously must not raise) and test_message_dispatch_task_is_tracked
    (the spawned handler task is held in _background_tasks while it runs and
    discarded on completion).

How to Test

  1. Run the adapter test file: pytest tests/gateway/test_qqbot.py -q — 163 passed.
  2. To confirm the tests catch the bug, revert just the dispatch line back to
    asyncio.create_task(self._on_message(t, d)): both new tests fail —
    test_message_dispatch_without_event_loop_does_not_raise raises
    RuntimeError: no running event loop, and test_message_dispatch_task_is_tracked
    asserts len(_background_tasks) == 1 but finds 0 because the bare task was
    never tracked.

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/gateway/test_qqbot.py -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 (Darwin 25.5.0)

Documentation & Housekeeping

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

…dropped

## What does this PR do?

The QQ Bot adapter spawned its inbound-message handler with a bare
`asyncio.create_task(self._on_message(t, d))` in `_dispatch_payload`. The
asyncio event loop only keeps a *weak* reference to a task created that way, so
with no strong reference held anywhere the garbage collector is free to cancel
the handler mid-flight — and when it does, the received QQ message is silently
dropped with no error. Every other branch in `_dispatch_payload` (Resume,
Identify, interaction, server-reconnect close) already routes through the
adapter's `_create_task` helper; only the message branch did not.

This routes the message branch through `_create_task` and makes that helper
hold each task in the inherited `self._background_tasks` set until it finishes
(discarding it via a done-callback), exactly as `BasePlatformAdapter` already
does for its own spawned tasks. As a side effect it also fixes a test-safety
gap: `_create_task` was created specifically so `_dispatch_payload` can be
called synchronously outside `asyncio.run()` without raising "no running event
loop", but the message branch bypassed it — so a message op was the one payload
type that could not be dispatched the way the existing synchronous tests
dispatch every other op.

## Related Issue

N/A

## Type of Change

- [x] 🐛 Bug fix (non-breaking change that fixes an issue)

## Changes Made

- `gateway/platforms/qqbot/adapter.py`: route the inbound-message dispatch
  (`C2C_MESSAGE_CREATE`, `GROUP_AT_MESSAGE_CREATE`, `DIRECT_MESSAGE_CREATE`,
  `GUILD_MESSAGE_CREATE`, `GUILD_AT_MESSAGE_CREATE`) through `self._create_task`
  instead of a bare `asyncio.create_task`.
- `gateway/platforms/qqbot/adapter.py`: change `_create_task` from a
  `@staticmethod` into an instance method that registers each spawned task in
  `self._background_tasks` and removes it again with an `add_done_callback`
  discard, so a still-running handler can't be garbage-collected. All call
  sites already invoked it as `self._create_task(...)`, so they are unchanged.
- `tests/gateway/test_qqbot.py`: add two tests in `TestDispatchPayload` —
  `test_message_dispatch_without_event_loop_does_not_raise` (a message op
  dispatched synchronously must not raise) and `test_message_dispatch_task_is_tracked`
  (the spawned handler task is held in `_background_tasks` while it runs and
  discarded on completion).

## How to Test

1. Run the adapter test file: `pytest tests/gateway/test_qqbot.py -q` — 163 passed.
2. To confirm the tests catch the bug, revert just the dispatch line back to
   `asyncio.create_task(self._on_message(t, d))`: both new tests fail —
   `test_message_dispatch_without_event_loop_does_not_raise` raises
   `RuntimeError: no running event loop`, and `test_message_dispatch_task_is_tracked`
   asserts `len(_background_tasks) == 1` but finds `0` because the bare task was
   never tracked.

## Checklist

### Code

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

### Documentation & Housekeeping

- [x] I've updated relevant documentation (README, `docs/`, docstrings) — updated the `_create_task` docstring
- [x] I've updated `cli-config.yaml.example` if I added/changed config keys — N/A
- [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` if I changed architecture or workflows — N/A
- [x] I've considered cross-platform impact (Windows, macOS) per the compatibility guide — pure asyncio, no platform-specific code
- [x] I've updated tool descriptions/schemas if I changed tool behavior — N/A
@daimon-nous daimon-nous Bot added type/bug Something isn't working platform/qqbot QQ Bot adapter comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists labels Jun 7, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the untracked QQ inbound dispatch path. The current-main premise is valid: gateway/platforms/qqbot/adapter.py:857 still uses a bare task while adjacent branches use _create_task.

Problems

  • gateway/platforms/qqbot/adapter.py:810 returns without closing coro when no loop exists. The changed message branch passes _on_message(...) at line 858, and _on_message is async on current main (gateway/platforms/qqbot/adapter.py:932), leaving an unawaited coroutine in the synchronous path. The existing base implementation closes this case at gateway/platforms/base.py:3067-3074.

Suggested changes

  • Close the coroutine before returning from the no-loop branch, and make test_message_dispatch_without_event_loop_does_not_raise exercise an async handler rather than a synchronous Mock.

Automated hermes-sweeper review.

loop = asyncio.get_running_loop()
return loop.create_task(coro)
except RuntimeError:
return None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coro was already created by the caller (including the new _on_message(...) path). Close it before returning here; otherwise synchronous dispatch leaves an unawaited coroutine. BasePlatformAdapter uses this exact cleanup pattern at gateway/platforms/base.py:3067-3074.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 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:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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.

2 participants