Skip to content

fix(tools): reuse persistent loop in Home Assistant async bridge#13422

Open
stepanov1975 wants to merge 2 commits into
NousResearch:mainfrom
stepanov1975:fix/ha-tool-persistent-async-bridge
Open

fix(tools): reuse persistent loop in Home Assistant async bridge#13422
stepanov1975 wants to merge 2 commits into
NousResearch:mainfrom
stepanov1975:fix/ha-tool-persistent-async-bridge

Conversation

@stepanov1975

@stepanov1975 stepanov1975 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

This PR fixes the remaining async-context gap in tools/homeassistant_tool.py's sync-to-async bridge.

The Home Assistant tool already reused persistent event loops for some sync call paths, but when _run_async() was invoked from inside an already-running event loop it still spawned a disposable thread and called asyncio.run(coro) per invocation. That per-call create-and-destroy loop behavior can leave aiohttp cleanup tied to dead loops, which risks Unclosed client session / Unclosed connector warnings in long-running Hermes processes.

This change makes that path reuse a dedicated persistent background loop thread instead, bringing the Home Assistant tool in line with Hermes' persistent-loop bridge pattern.

Related Issue

Fixes #13586

Type of Change

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

Changes Made

  • update tools/homeassistant_tool.py so async-context calls no longer use per-call asyncio.run()
  • add a dedicated persistent background loop thread for calls made while another event loop is already running
  • submit async-context coroutines with asyncio.run_coroutine_threadsafe(...)
  • keep the existing persistent main-thread and per-worker-thread loop behavior
  • add regression coverage in tests/tools/test_homeassistant_tool.py for loop reuse and liveness from inside a running event loop

How to Test

  1. Reproduce the bug scenario by calling tools/homeassistant_tool.py:_run_async() from within an already-running event loop.
  2. Verify that the call succeeds without falling back to per-call asyncio.run() loop creation/destruction behavior.
  3. Run the targeted regression tests:
    • python -m pytest tests/tools/test_homeassistant_tool.py -q -o 'addopts='
    • python -m pytest tests/gateway/test_homeassistant.py -q -o 'addopts='
  4. Optional broader check: run pytest tests/ -q.

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

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

Targeted local test results:

  • tests/tools/test_homeassistant_tool.py: passed
  • tests/gateway/test_homeassistant.py: passed

Note: I did not mark the full pytest tests/ -q checkbox as complete here because repository-wide baseline failures unrelated to this PR were observed separately.

@stepanov1975 stepanov1975 force-pushed the fix/ha-tool-persistent-async-bridge branch 2 times, most recently from 3fca7db to df04c30 Compare April 21, 2026 15:13
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets labels Apr 22, 2026
@stepanov1975

stepanov1975 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

@0xbyt4 sorry for the direct ping — I’m trying not to create review noise, but this PR has been sitting without checks because it needs maintainer workflow approval.

I believe the underlying issue is still present on current main: tools/homeassistant_tool.py::_run_async() still uses the disposable ThreadPoolExecutor + asyncio.run(coro) path when called from inside an already-running event loop, which this PR replaces with persistent loop reuse.

If this PR fix not good enough, may be you can look at the issue and create better fix as issue still need fix.
Sorry for any inconvenience caused by this ping, I am new to all this.

Keep homeassistant_tool._run_async() on persistent event loops across main-thread, worker-thread, and already-running-loop call modes. Replace the remaining per-call asyncio.run() path with a background bridge loop and add regression coverage for running-loop reuse.
@stepanov1975 stepanov1975 force-pushed the fix/ha-tool-persistent-async-bridge branch from df04c30 to 2f9f232 Compare June 8, 2026 13:12
@stepanov1975 stepanov1975 force-pushed the fix/ha-tool-persistent-async-bridge branch from 2f9f232 to eb104e9 Compare June 8, 2026 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Home Assistant tool still uses per-call asyncio.run() path inside running event loops

2 participants