Skip to content

fix(web-server): move event channel state from module globals to app.state - #37683

Merged
OutThisLife merged 1 commit into
mainfrom
ethie/fix-test-web-server-flake
Jun 2, 2026
Merged

fix(web-server): move event channel state from module globals to app.state#37683
OutThisLife merged 1 commit into
mainfrom
ethie/fix-test-web-server-flake

Conversation

@ethernet8023

Copy link
Copy Markdown
Collaborator

What does this PR do?

Moves _event_channels and _event_lock out of module-level globals in hermes_cli/web_server.py and onto app.state, initialised via a FastAPI lifespan context manager. This eliminates the class of test flake where asyncio.Lock() created at import time binds to an event loop that is no longer running when a later TestClient (or uvicorn reload) tries to use it.

Root cause: asyncio.Lock() captures the currently-running event loop at construction time. A module-level lock created during import binds to whatever loop was active then. When the module is reused in a new TestClient or after a uvicorn reload, the old lock still references a defunct loop, raising attached to a different loop errors and causing subscriber-registration races under parallel test execution.

Approach:

  • Add _lifespan() async context manager — creates event_channels dict + event_lock on the running loop during FastAPI startup
  • Add _get_event_state(app) lazy accessor — falls back to on-demand init when TestClient is used without a with block, preserving backward compat for existing non-lifespan test setups
  • All call sites (_broadcast_event, /api/pub, /api/events) now receive the app reference and read state via _get_event_state(app)
  • Test updated to poll app.state.event_channels instead of the removed module-level _event_channels

Related Issue

Fixes recurring CI flake in test_web_server.py subscriber-registration tests.

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

  • hermes_cli/web_server.py — Replace module-level _event_channels/_event_lock with app.state-based state; add _lifespan() and _get_event_state(); update all consumers to pass app explicitly
  • tests/hermes_cli/test_web_server.py — Update polling assertion from ws_mod._event_channels to ws_mod.app.state.event_channels

How to Test

  1. Run the web server subscriber tests in isolation: python -m pytest tests/hermes_cli/test_web_server.py -q -k broadcast
  2. Run the full web server test suite: python -m pytest tests/hermes_cli/test_web_server.py -q
  3. Verify no attached to a different loop errors appear across multiple sequential TestClient usages

Checklist

Code

  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this is not a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I have added tests for my changes (existing tests updated to match new state layout)
  • I have tested on my platform: NixOS

Documentation & Housekeeping

  • N/A — internal refactor, no API or config changes

…state

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

🔎 Lint report: ethie/fix-test-web-server-flake vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9682 on HEAD, 9682 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 5016 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard labels Jun 2, 2026
@OutThisLife
OutThisLife merged commit cbc8251 into main Jun 2, 2026
23 checks passed
@OutThisLife
OutThisLife deleted the ethie/fix-test-web-server-flake branch June 2, 2026 23:40
Yuki-14544869 pushed a commit to Yuki-14544869/hermes-agent that referenced this pull request Jun 4, 2026
…state (NousResearch#37683)

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
davidgut1982 pushed a commit to davidgut1982/hermes-agent that referenced this pull request Jun 5, 2026
…state (NousResearch#37683)

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
changman pushed a commit to changman/hermes-agent that referenced this pull request Jun 10, 2026
…state (NousResearch#37683)

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
alt-glitch pushed a commit that referenced this pull request Jun 14, 2026
…state (#37683)

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
kossteg pushed a commit to kossteg/hermes-agent that referenced this pull request Jun 16, 2026
…state (NousResearch#37683)

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
T02200059 pushed a commit to T02200059/hermes-agent that referenced this pull request Jun 18, 2026
…state (NousResearch#37683)

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
xyshanren pushed a commit to xyshanren/hermes-agent-cn that referenced this pull request Jun 25, 2026
…state (NousResearch#37683)

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…state (NousResearch#37683)

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…state (NousResearch#37683)

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
donbowman pushed a commit to donbowman/hermes-agent that referenced this pull request Jul 13, 2026
…state (NousResearch#37683)

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…state (NousResearch#37683)

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…state (NousResearch#37683)

Module-level asyncio.Lock() binds to whatever event loop was active at
import time.  When the same web_server module is reused across multiple
TestClient instances (or across uvicorn reloads), the old lock still
references a defunct loop, causing 'attached to a different loop' errors
and flaky subscriber-registration races in CI.

Replace the module-level _event_channels dict + _event_lock with:
  - _lifespan() async context manager that creates both on the running
    event loop during FastAPI startup (guaranteed correct loop binding)
  - _get_event_state() lazy accessor that initialises on app.state when
    TestClient is used without a `with` block (preserves backward compat)

All call sites (_broadcast_event, /api/pub, /api/events) now receive the
app reference and read state via _get_event_state(app) instead of the
module globals.  The test polling loop is updated to check
app.state.event_channels rather than the removed module attribute.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard 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.

3 participants