fix(weixin): Replace cross-loop session reuse with thread-local persistent session#18251
Open
ikelvingo wants to merge 2 commits into
Open
fix(weixin): Replace cross-loop session reuse with thread-local persistent session#18251ikelvingo wants to merge 2 commits into
ikelvingo wants to merge 2 commits into
Conversation
Collaborator
Collaborator
Contributor
Author
|
Because in version v2026.4.23, a PR was merged that is neither secure nor compliant with aiohttp's usage guidelines, and this was also not fixed in version v2026.4.30. |
f998cad to
6fca9a3
Compare
…stent session Port of NousResearch#18251: - model_tools.py: add _get_worker_session() with thread-local persistent aiohttp.ClientSession bound to each worker thread's event loop - gateway/platforms/weixin.py: remove _LIVE_ADAPTERS lazy-reuse block in send_weixin_direct(), use _get_worker_session() instead fix(honcho): sync_turn handles multimodal (list) content from image-bearing messages Port of NousResearch#22768: - plugins/memory/honcho/__init__.py: expand type annotation to str | list for user_content and assistant_content; add _extract_text() helper to extract text parts from multimodal lists before sanitize_context() - image references are excluded from memory (memory providers should not store image URIs)
Port of upstream f24b7ed (fix: make Honcho startup fail open): - plugins/memory/honcho/__init__.py: refactor startup to background thread via _start_session_init_background(), add _session_ready() guard, and new _init_thread/_init_lock/_init_error fields for fail-open behavior - tests/test_honcho_startup_fail_open.py: add 357-line test suite Port of NousResearch#22768 (fix(honcho): sync_urn crashes on list input from image-bearing messages): - plugins/memory/honcho/__init__.py: expand sync_urn type annotation to str | list for user_content and assistant_content; add _extract_text() helper to extract text parts from multimodal lists; image references are excluded from memory (memory providers should not store image URIs)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Symptom
send_messagetool fails consistently with:RuntimeError: Timeout context manager should be used inside a taskaiohttp.TimerContext.current_task()returnsNonewhen the session was created on a different event loop than the one callingsession.post().Root Cause
send_weixin_direct().aiohttp.ClientSession(from_LIVE_ADAPTERS) for one-shot sends triggered by thesend_messagetool.ThreadPoolExecutorworker thread via_run_async().ClientSessionwas created on the gateway's event loop, butsession.post(timeout=...)was called from the worker thread — a cross-event-loop access thataiohttp.TimerContextdoes not support.TimerContext.__enter__()callsasyncio.current_task(loop=session._loop). Since no task is running on the gateway's loop inside the worker thread, it returnsNone, raising theRuntimeError.Intent of the Original Change
send_messagetool invocation.Risks of the Original Approach
aiohttp.ClientSessionis explicitly documented as not thread-safe.Type of Change
Changes Made
Fix
Added
_get_worker_session()tomodel_tools.py:aiohttp.ClientSessionviathreading.local()._get_worker_loop()), soTimerContext.current_task()always finds a valid task._make_ssl_connector()fromgateway/platforms/weixin.py.Removed lazy recreate branch from
send_weixin_direct():_LIVE_ADAPTERS.get(resolved_token)shortcut (26 lines).async with aiohttp.ClientSession(trust_env=True, connector=...)withsession = _get_worker_session().Behavior After Fix
TimerContext.current_task()always returns the running task on the worker's loop.Conflict Resolution
During review, a parallel approach was identified: replacing
ClientTimeoutwithasyncio.wait_for()(chenlinfeng, May 3). This masks theTimerContexterror but leaves the underlyingClientSessionthread-safety violations intact.TimerContexterrorClientSessionthread-safetyThe thread-local session approach eliminates all root-cause risks simultaneously.
asyncio.wait_for()only replaces the error type (TimeoutErrorvsRuntimeError) without fixing the architectural problem.Files Changed
model_tools.py(+13 lines): Added_get_worker_session()with thread-local persistent session management.gateway/platforms/weixin.py(-21 lines): Removed lazy recreate block; replacedasync with ClientSessionwith persistent worker session getter.How to Test
Verification
send_messagetool with WeChat image attachments: ✅success: true, validmessage_idreturned.RuntimeErrororsend failedentries post-fix.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
hermes --toolsets skills -q "Use the X skill to do Y"Screenshots / Logs