Skip to content

fix(weixin): avoid cross-loop live adapter reuse in direct send#13361

Closed
dso2ng wants to merge 1 commit into
NousResearch:mainfrom
dso2ng:fix/weixin-cross-loop-send-direct
Closed

fix(weixin): avoid cross-loop live adapter reuse in direct send#13361
dso2ng wants to merge 1 commit into
NousResearch:mainfrom
dso2ng:fix/weixin-cross-loop-send-direct

Conversation

@dso2ng

@dso2ng dso2ng commented Apr 21, 2026

Copy link
Copy Markdown

Summary

  • stop send_weixin_direct() from reusing a live Weixin adapter across event loops
  • force one-shot Weixin sends to use an isolated aiohttp.ClientSession
  • add a regression test covering cross-loop direct-send safety

Why

Cron delivery and other one-shot messaging paths can call send_weixin_direct() outside the gateway's main event loop. Reusing a live Weixin adapter from those paths can cross event-loop boundaries and trigger aiohttp task/timeout errors instead of delivering the message.

Root cause

send_weixin_direct() preferred _LIVE_ADAPTERS[resolved_token] whenever a live adapter and _send_session existed. That live adapter's aiohttp session is created inside the gateway event loop, but one-shot delivery helpers may execute from another loop or thread. Reusing the live adapter directly in that context can raise errors such as:

  • Timeout context manager should be used inside a task

Behavior before

  • cron / one-shot Weixin delivery could try to reuse a live adapter bound to another event loop
  • delivery could fail with aiohttp timeout/task-context errors

Behavior after

  • send_weixin_direct() no longer reuses a live adapter across loops
  • one-shot delivery uses an isolated session instead
  • live gateway operation remains unchanged for normal in-loop sends

How to test

  • source venv/bin/activate && python -m pytest tests/gateway/test_weixin.py -q
  • with the gateway running and a Weixin target configured, trigger cron or one-shot delivery and confirm it no longer fails with Timeout context manager should be used inside a task

Platforms tested

  • Linux (local Hermes development environment on omarchy)

Notes

  • This PR is intentionally scoped to event-loop safety for one-shot Weixin delivery helpers.
  • It takes the low-risk approach of avoiding cross-loop live-adapter reuse instead of adding more complex loop-handoff logic.

@dso2ng

dso2ng commented Apr 21, 2026

Copy link
Copy Markdown
Author

Maintainer context: this PR is intended to address the live Weixin cron / one-shot delivery failure pattern that surfaces as:

  • Weixin send failed: Timeout context manager should be used inside a task

Local verification is complete:

  • source venv/bin/activate && python -m pytest tests/gateway/test_weixin.py -q

The change is intentionally narrow:

  • it avoids reusing a live Weixin adapter across event loops in send_weixin_direct()
  • and instead forces the one-shot helper path to use its own isolated session
  • with a regression test covering the cross-loop reuse hazard

If fork workflows are still blocked in action_required, a maintainer approval / run from the GitHub UI will likely be needed before CI can actually start.

@dso2ng

dso2ng commented Apr 21, 2026

Copy link
Copy Markdown
Author

@kshitijk4poor @teknium1 friendly ping on this Weixin fix: it is narrowly scoped to the cross-event-loop direct-send failure pattern (Timeout context manager should be used inside a task) and has a dedicated regression test plus a clean local tests/gateway/test_weixin.py pass.

If fork workflows still need maintainer approval to run, approving/running them in the GitHub UI should let CI start and make the remaining signal much clearer.

@dso2ng dso2ng force-pushed the fix/weixin-cross-loop-send-direct branch from 630034a to 056f9d5 Compare April 21, 2026 08:21
@dso2ng

dso2ng commented Apr 21, 2026

Copy link
Copy Markdown
Author

Maintainer note: I updated this PR to use my GitHub noreply commit email, so the attribution issue should be fixed. The current checks are action_required with zero jobs started, so it looks like fork-PR workflow approval is blocking CI from actually running. Once approved/run in the GitHub UI, I can follow up on any real failures.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the contribution, @dso2ng!

Closing this as a duplicate of #13350 (by @ifnodoraemon), which targets the same fix/feature. We're consolidating on that PR for review.

If you want to help push it over the line, please jump in there — or if you think your approach is better for a specific reason that isn't covered in the other PR, let us know and we can reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants