Skip to content

fix(feishu): isolate lark_oapi WS globals per profile and supervise the client thread - #84165

Open
pittosporum-seu wants to merge 1 commit into
NousResearch:mainfrom
pittosporum-seu:fix/feishu-multiplex-loop-isolation-73779
Open

fix(feishu): isolate lark_oapi WS globals per profile and supervise the client thread#84165
pittosporum-seu wants to merge 1 commit into
NousResearch:mainfrom
pittosporum-seu:fix/feishu-multiplex-loop-isolation-73779

Conversation

@pittosporum-seu

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the multiplex-mode failure where Feishu profiles either crash with Future attached to a different loop or go deaf from the start because all profiles fight over two process-wide globals, and adds a supervisor so a dead WS client thread no longer leaves a profile silently deaf until a gateway restart.

Root cause (verified against lark-oapi 1.6.8, lark_oapi/ws/client.py):

  • The SDK keeps the asyncio loop used by Client.start() and every coroutine it spawns (_ping_loop, _receive_message_loop, _handle_message, reconnects) in a module-level global (loop, created at import time).
  • Hermes' _run_official_feishu_ws_client therefore assigns ws_client_module.loop = <own loop> per profile thread and monkey-patches ws_client_module.websockets.connect (which patches the shared websockets module) to inject per-adapter ping settings.

In multiplex mode every profile runs its own WS client on a dedicated thread, so the N threads overwrite each other's globals (last-write-wins):

  • a client schedules tasks on a sibling profile's loop → Future attached to a different loop crashes, and
  • a client binds to the wrong loop at construction time → the profile receives nothing ever again ("deaf from birth"), matching the three-way reproduction in the issue (second profile deaf from start, restart does not help, separate processes work).

The fix installs process-wide, thread-dispatching shims exactly once:

  • ws_client_module.loop becomes a proxy that forwards every attribute access to the loop registered by the calling thread. This is sound because all SDK reads of the global happen on the thread that owns the loop (start() blocks in run_until_complete; every create_task callback runs on the loop's own thread). Threads that never registered one (single-profile installs, CLI) fall back to the SDK's original module loop — behavior is unchanged there.
  • websockets.connect becomes a single dispatcher merging the per-thread ping overrides registered by the calling profile, so profiles no longer race over the global patch or restore each other's hooks while a sibling is still connected. __wrapped__ keeps inspect.signature(websockets.connect) honest for the SDK's _ws_connect_kwargs() websockets-15 proxy probe.
  • If the shims cannot install (unexpected SDK layout), the code falls back to the legacy direct-assignment path so startup never breaks.

Supervised reconnect: lark_oapi's start() blocks forever on a healthy connection and only returns on fatal errors; until now the executor future was awaited solely by disconnect(), so a dead thread left the profile silently deaf. The adapter now watches the future and, on unexpected exit while it is supposed to be connected, rebuilds the client with capped exponential backoff (5s → 60s).

Related Issue

Fixes #73779

Related: #64247, #53477 (supervised-reconnect alone only covers the crash variant; this PR covers both variants plus the silent-death case).

Type of Change

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

Changes Made

  • plugins/platforms/feishu/adapter.py:
    • new _ThreadLocalLoopProxy + _install_lark_ws_isolation() (idempotent, lock-guarded, per-thread threading.local registry)
    • _run_official_feishu_ws_client(): register per-thread loop + connect overrides instead of assigning shared globals; legacy fallback retained
    • new _supervise_websocket_thread(): watches _ws_future, restarts the client with capped backoff on unexpected thread exit; spawned by connect() in websocket mode, cancelled by disconnect()
  • tests/gateway/test_feishu_ws_multiplex_isolation.py: 12 tests (per-thread loop dispatch under concurrency, fallback, idempotency, connect-dispatcher semantics incl. signature probe, isolated/legacy run paths with cleanup, supervised restart incl. deliberate-disconnect and failed-restart backoff)

How to Test

  1. pytest tests/gateway/test_feishu_ws_multiplex_isolation.py -q — 12 passed.
  2. pytest tests/gateway/ -k feishu -q — 216 passed, 1 skipped (no regressions); ruff check clean.
  3. Multiplex soak: run a gateway with ≥2 Feishu profiles in one process; before the fix the later profiles intermittently crash with Future attached to a different loop or never receive events; after the fix each thread schedules on its own loop. I will additionally run this against a real 7-profile gateway (offered in the issue thread).

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: Ubuntu 24.04 (WSL2)

Note: ran the full Feishu gateway suite (tests/gateway/ -k feishu, 216 passed) plus the new isolation tests and ruff check; the change is contained to the Feishu websocket path.

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

Failure signatures this PR eliminates (live multiplex gateway, one process, 7 profiles):

ERROR agent.conversation_loop: ... RuntimeWarning: coroutine ... was never awaited / Future attached to a different loop
(gateway alive, profile connected, but no inbound events are ever delivered on the affected profile)

…he client thread

lark_oapi.ws.client keeps the asyncio loop used by Client.start() and all of its coroutines in a module-level global, and Hermes monkey-patches websockets.connect on the shared websockets module. In multiplex mode every profile runs its own WS client on a dedicated thread, so the N threads overwrite each other's module globals (last-write-wins): a client schedules tasks on a sibling profile's loop ('Future attached to a different loop' crashes) or binds to the wrong loop at construction time and goes deaf from the start (NousResearch#73779). Install process-wide thread-dispatching shims once: the module loop becomes a proxy forwarding to the calling thread's registered loop (all SDK reads happen on the loop's owning thread), and websockets.connect becomes a dispatcher merging per-thread ping overrides. Falls back to the legacy direct-assignment path if the shims cannot install. Also add a supervisor: the executor future was previously awaited only by disconnect(), so a dead WS thread left the profile silently deaf; now the adapter watches it and rebuilds the client with capped exponential backoff while it is supposed to be connected.
@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins platform/feishu Feishu / Lark adapter area/config Config system, migrations, profiles P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 12, 2026
@pittosporum-seu

Copy link
Copy Markdown
Contributor Author

Soak test on a real 7-profile multiplex gateway (as offered in #73779)

Ran the exact code from this branch as a live gateway for ~35 minutes against a production-configured Hermes install that runs 7 Feishu profiles in one process (multiplex mode) — the topology this fix targets.

Setup

  • Branch tree copied to a native-FS path; run with the production venv + production HERMES_HOME. Import check first confirmed hermes_cli / plugins.platforms.feishu.adapter / gateway.run all resolved to the soak copy and that _install_lark_ws_isolation + _supervise_websocket_thread were present.
  • Stopped the production gateway (which was running a different local hotfix for the same issue), started the soak gateway in its place.

Results

  • All 7 Feishu profiles connected in websocket mode; Gateway running with 7 platform(s).
  • Zero Future attached to a different loop errors for the whole window (pre-window baseline also verified 0).
  • Zero adapter tracebacks (the only traceback in the log is the harmless optional nemo_relay import note, which production logs too).
  • Cron scheduler ticked 8+ times across the 7 profiles; every turn resolved model/provider normally.
  • One real inbound message processed end-to-end: inbound -> conversation turn -> final delivery, no loop errors.

Caveat

  • A second message aimed at a different profile did not arrive inside the window, so concurrent multi-profile receive is evidenced by 7 simultaneous WS connections plus per-profile cron turns rather than simultaneous inbound messages.

Rollback afterwards: production gateway restored, 7/7 profiles reconnected cleanly.

Environment: lark-oapi 1.6.8, Python 3.12, Ubuntu (WSL2).

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(feishu): isolate lark_oapi WS globals per profile and supervise the client thread

  1. plugins/platforms/feishu/adapter.py_ThreadLocalLoopProxy only forwards attribute access; isinstance(loop, asyncio.AbstractEventLoop) and any identity comparison against the real loop will fail against the proxy. The legacy fallback covers install failures, not runtime type checks in the SDK (now or in a future lark_oapi version). Worth a comment documenting the constraint and failure mode so a future SDK change is recognized quickly.

  2. _supervise_websocket_thread awaits self._connect_websocket() directly on the gateway event loop. If _connect_websocket blocks (executor submission, connect/DNS latency) it stalls the loop, and on failure self._ws_future still references the dead future, so the supervisor immediately re-loops with only the backoff sleep between iterations. Confirm _connect_websocket is non-blocking and atomically replaces _ws_future.

  3. When _install_lark_ws_isolation fails in multiplex mode, the legacy fallback reintroduces exactly the cross-profile global race this PR fixes — but only logs a warning. Consider logging at ERROR level when multiplex_profiles is enabled, so a deployment that silently lost isolation is visible in logs.

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

Labels

area/config Config system, migrations, profiles comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/feishu Feishu / Lark adapter 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.

Feishu multiplex mode: lark_oapi WebSocket receive loop dies with Future attached to a different loop, gateway silently stops receiving messages

3 participants