Skip to content

fix(mattermost): use per-call aiohttp ClientSession to avoid cross-loop bug - #35343

Closed
idarius wants to merge 1 commit into
NousResearch:mainfrom
idarius:fix/mattermost-cross-loop-session
Closed

fix(mattermost): use per-call aiohttp ClientSession to avoid cross-loop bug#35343
idarius wants to merge 1 commit into
NousResearch:mainfrom
idarius:fix/mattermost-cross-loop-session

Conversation

@idarius

@idarius idarius commented May 30, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes Plugin platform send failed: Timeout context manager should be used inside a task raised whenever an agent invokes send_message (or any cross-channel send) into a Mattermost channel from inside the gateway event loop.

Root cause

MattermostAdapter.connect() cached one aiohttp.ClientSession on self._session, bound to the gateway's main event loop. Every REST helper reused that cached session.

When send_message is dispatched from the agent, model_tools._run_async detects the running gateway loop and spins up a fresh event loop on a worker thread to run the coroutine — see model_tools.py lines 84–143. The cached session is consumed cross-loop, and aiohttp.helpers.TimerContext.__enter__ raises:

RuntimeError: Timeout context manager should be used inside a task

aiohttp's check is asyncio.current_task(loop=self._loop) against the session's loop; the worker-loop bridge has no current task in the gateway loop, so the check fails.

The bug exists since the Mattermost plugin was created; it's been latent because most replies go through the gateway dispatch path (not the send_message tool). It surfaces consistently for cron deliveries and agent-initiated cross-channel sends.

Related Issue

Fixes the runtime described above. No tracking issue was open for Mattermost specifically. Same architectural class as the weixin reports #33445, #31853, #17468, #17267, #18417, #28835, #35237 (Mattermost variant only here).

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • plugins/platforms/mattermost/adapter.py:
    • Add _open_session(timeout_sec) — returns a context manager for a fresh aiohttp.ClientSession bound to the current event loop. Test fixtures that pre-load self._session = MagicMock() are routed through _PassthroughSessionCM so existing tests keep intercepting HTTP calls without modification.
    • Refactor 8 HTTP call sites to use _open_session instead of the cached self._session.<method>(...): _api_get, _api_post, _api_put, _upload_file, _send_url_as_file, the inline downloads in send_multiple_images, and the file-download path in _handle_ws_event.
    • Drop the default ClientTimeout from self._session = aiohttp.ClientSession() in connect() — the cached session is now used only for the WebSocket, which uses heartbeat= for liveness.

+124 / -79 lines, no public-API changes, no new dependencies.

Same architectural pattern as #1736 (fix(gateway): SMS session-per-send, merged) and #3818 (fix(whatsapp): reuse persistent aiohttp session across requests, merged).

How to Test

  1. Set up a Mattermost server with a bot account, configure MATTERMOST_URL and MATTERMOST_TOKEN.
  2. Start the gateway: hermes gateway start.
  3. In any monitored channel, ask the agent to post into another channel via the send_message tool. Example:
    @hermes please send "test" to mattermost:<another-channel-id>
    
  4. Before the fix: the gateway logs WARNING agent.tool_executor: Tool send_message returned error … "Plugin platform send failed: Timeout context manager should be used inside a task" and the agent never delivers.
  5. After the fix: the message is posted, the tool returns {"success": true, "message_id": "..."}.

Out-of-process and gateway-loop paths still work unchanged:

  • hermes send -t mattermost "test" (out-of-process) — unchanged.
  • File downloads triggered from incoming Mattermost posts (gateway-loop path) — unchanged.
cd hermes-agent
./venv/bin/python -m pytest tests/gateway/test_mattermost.py -q
# 43 passed
Before After
send_message tool from agent context Plugin platform send failed: Timeout context manager… (every call) success
_handle_ws_event file download (gateway loop) success success (unchanged behaviour)
hermes send -t mattermost "…" out-of-process success success (unchanged behaviour)
Mattermost adapter tests 43 pass 43 pass (no test changes)

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 — none for Mattermost
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/gateway/test_mattermost.py -q and all tests pass
  • I've added tests for my changes — N/A: existing tests cover the refactored code paths, no behavioural surface added
  • I've tested on my platform: Ubuntu 24.04 / Python 3.11 in the bundled venv

Documentation & Housekeeping

  • I've updated relevant documentation — N/A (internal helper, no public-facing change)
  • 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 — N/A (pure asyncio, no filesystem or shell)
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

…op bug

The Mattermost adapter cached an aiohttp.ClientSession in connect() and
reused it across every REST helper. When send_message is dispatched from
the agent via model_tools._run_async — which runs the coroutine in a
fresh event loop on a worker thread — the cached session is consumed
cross-loop, and aiohttp.helpers raises:

    RuntimeError: Timeout context manager should be used inside a task

Replace the eight HTTP call sites with a new _open_session(timeout_sec)
helper that creates a per-call ClientSession bound to the current event
loop. self._session is kept for the WebSocket only (same loop as
connect(), no cross-loop issue).

The helper short-circuits when self._session is a non-aiohttp object
(test mocks), routing through _PassthroughSessionCM so the 43 existing
Mattermost tests keep intercepting HTTP calls unchanged.

Pattern follows #1736 (session-per-send for SMS, merged).
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins labels May 30, 2026
@idarius idarius closed this by deleting the head repository Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants