Skip to content

fix(gateway): follow session rotation in the heartbeat poller - #80208

Open
0xGr1mm wants to merge 1 commit into
NousResearch:mainfrom
0xGr1mm:fix/gateway-heartbeat-follows-session-rotation
Open

fix(gateway): follow session rotation in the heartbeat poller#80208
0xGr1mm wants to merge 1 commit into
NousResearch:mainfrom
0xGr1mm:fix/gateway-heartbeat-follows-session-rotation

Conversation

@0xGr1mm

@0xGr1mm 0xGr1mm commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

/heartbeat stops firing permanently the first time a gateway session compresses, and nothing tells the user.

_register_heartbeat_watch stores quick_key -> (source, session_id), and the poller used that captured session_id on every tick. A context compression rotates the route onto a fresh session and carries the heartbeat across with migrate_heartbeat_to_session, which writes the state to the child and marks the parent row cleared. The poller is still holding the parent id, so it reads the cleared row, reads it as "the user removed the heartbeat", and unregisters the watch:

mgr = HeartbeatManager(session_id=session_id)   # parent id — cleared by the migration
if not mgr.has_heartbeat():
    watch.pop(quick_key, None)                  # gone for the life of the process
    continue

The failure is silent in the worst way: /heartbeat status goes through _get_heartbeat_manager_for_event, which resolves the live session, finds the migrated state, and keeps reporting a healthy heartbeat with a ticking countdown. So the one surface a user would check to diagnose it actively confirms everything is fine.

Reproduced against the real state layer (migrate_heartbeat_to_session + HeartbeatManager, with the poller's decision replayed verbatim):

poll before compression : FIRED
state on OLD sid        : None
state on NEW sid        : present
poll after compression  : WATCH DROPPED - heartbeat never fires again
watch registry now      : {}

/heartbeat status says  : ♥ Heartbeat (every 10m, next in ~599s, fired 1×): Check the deploy

The CLI driver does not have this bug — _get_heartbeat_manager rebinds whenever session_id changes, so it follows the rotation already. This PR restores the same property on the gateway.

The fix

Re-resolve the route's current session id on every tick through a new _live_heartbeat_session_id, and cache the result back into the registry so the next tick starts from the live id.

SessionStore.peek_session_id is the right accessor here: it is the public, lock-held, read-only view of the same key→session_id mapping that advance_compression_session repairs during rotation, and quick_key is exactly the session_key that mapping is keyed by (_session_key_for_source_generate_session_key, the same value passed to advance_compression_session). It never creates a session, so a route the user has since ended resolves to nothing, falls back to the captured id, finds no heartbeat, and drops the watch exactly as before.

Related Issue

No existing issue. Searched open and merged PRs and issues first, per CONTRIBUTING's search-first section:

gh search prs --repo NousResearch/hermes-agent "heartbeat compression session"   # 0
gh search prs --repo NousResearch/hermes-agent "heartbeat poller session_id"     # 0
gh search prs --repo NousResearch/hermes-agent "migrate_heartbeat_to_session"    # 0
gh search prs --repo NousResearch/hermes-agent "_register_heartbeat_watch"       # only #80185 (mine)
gh search issues --repo NousResearch/hermes-agent "heartbeat stops firing"       # 0

Bug was introduced with /heartbeat in #79681.

Type of Change

Bug fix (non-breaking).

Changes Made

  • gateway/run.py — added _live_heartbeat_session_id(quick_key, fallback); the poll loop resolves through it and writes the resolved id back into _heartbeat_watch; updated the _register_heartbeat_watch docstring so the stored id is documented as a starting point rather than the source of truth.
  • tests/gateway/test_heartbeat_poller_session_rotation.py — new, 6 tests: the rotation regression, the two unchanged-behaviour paths (cleared heartbeat still unregisters, busy route still coalesces), and three _live_heartbeat_session_id cases (live mapping wins, unknown route falls back, raising store falls back).

How to Test

pytest tests/gateway/test_heartbeat_poller_session_rotation.py -q

6 passed.

The regression test drives the real _start_heartbeat_poller loop against a bare GatewayRunner, with the route mapping mutated mid-test the way a real rotation mutates the store. Reverting only gateway/run.py and rerunning:

FAILED test_poller_follows_compression_rotation - heartbeat never fired after the session rotated
FAILED test_live_session_id_prefers_the_route_mapping
FAILED test_live_session_id_falls_back_for_unknown_route
FAILED test_live_session_id_falls_back_when_the_store_raises
4 failed, 2 passed

The two that still pass without the fix are the unchanged-behaviour tests, which is the point — they pin that this change does not alter the drop path or the busy-coalesce path.

End to end: set /heartbeat every 60s … on a gateway platform, drive the session until it compresses, and confirm the heartbeat keeps arriving. Before this change it goes quiet at the rotation while /heartbeat status keeps counting down.

Full gateway suite, this branch vs main, same environment:

main:   7 failed, 4847 passed, 30 skipped, 1 xfailed
branch: 7 failed, 4853 passed, 30 skipped, 1 xfailed

The failing sets are identical — zero added, zero removed — and those 7 are pre-existing in a non-hermetic single-process run (pytest tests/gateway/); CI's per-file isolation via run_tests_parallel.py is the supported path.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(gateway): …)
  • I searched for existing PRs to make sure this isn't a duplicate (queries above)
  • My PR contains only changes related to this fix
  • I've run the affected suites and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: macOS 15 (Darwin 25.5), Python 3.11

Documentation & Housekeeping

  • I've updated relevant documentation — docstrings on the new helper and on _register_heartbeat_watch; the user-facing heartbeat.md needs no change because it describes the behaviour this restores
  • cli-config.yaml.example — N/A, no config keys
  • CONTRIBUTING.md / AGENTS.md — N/A
  • Cross-platform impact — N/A, no paths or platform APIs
  • Tool descriptions/schemas — N/A, /heartbeat is a slash command

Notes for the reviewer

peek_session_id is a synchronous, lock-held call made from the poll task. That matches what the loop already does — it constructs a HeartbeatManager on the same tick, which reads SQLite synchronously — and it is the lighter of the two. If you would rather the whole tick move off the loop, say so and I will wrap both in asyncio.to_thread in this PR.

One deliberate non-change: a route reset with /new (a switch_session, not a compression) does not migrate heartbeat state, so the new session has none and the watch is dropped. That reads as correct for a session-scoped feature — /new is a fresh conversation — so I left it alone rather than widen the scope here.

Stacked on the same feature area as #80185 (/heartbeat interval parsing) but independent: different files, no overlap, either can merge first.

Unrelated observation from verifying this, in case it is news: gateway.status._get_lock_dir() resolves to a machine-global path ($XDG_STATE_HOME/hermes/locks, i.e. ~/.local/state/hermes/locks) and tests/conftest.py does not sandbox HERMES_GATEWAY_LOCK_DIR the way it sandboxes HERMES_HOME. So the gateway platform-lock tests write to the developer's real lock directory, and two concurrent pytest tests/gateway/ runs on one machine collide — test_whatsapp_connect.py::TestDataInitialized::test_no_name_error_when_json_always_fails fails with "WhatsApp session already in use". I hit exactly that while running a branch and a baseline suite side by side; serially both are clean. Not touched here — flagging it in case it is worth a conftest sandbox.

`/heartbeat` stops firing permanently, and silently, the first time a
gateway session compresses.

`_register_heartbeat_watch` stores `quick_key -> (source, session_id)` and
the poller used that captured `session_id` on every tick. A context
compression rotates the route onto a fresh session and carries the
heartbeat across with `migrate_heartbeat_to_session`, which writes the
state to the child and marks the parent row `cleared`. The poller, still
holding the parent id, read the cleared row, took it for "user removed the
heartbeat", and dropped the watch:

    mgr = HeartbeatManager(session_id=session_id)   # parent id, now cleared
    if not mgr.has_heartbeat():
        watch.pop(quick_key, None)                  # gone for good
        continue

Nothing surfaces it. `/heartbeat status` resolves the *live* session via
`_get_heartbeat_manager_for_event`, finds the migrated state, and keeps
reporting `♥ Heartbeat (every 10m, next in ~599s)` for a heartbeat that
will never fire again in that process. Reproduced against the real state
layer: firing before rotation, watch dropped after it, status still
counting down.

The CLI driver is unaffected — `_get_heartbeat_manager` rebinds whenever
`session_id` changes, so it follows the rotation already. This restores the
same property on the gateway.

Re-resolve the route's current session id each tick through the new
`_live_heartbeat_session_id`, and cache it back into the registry.
`SessionStore.peek_session_id` is the read-only, lock-held accessor for the
key→session_id mapping that `advance_compression_session` repairs on
rotation; `quick_key` is the same `session_key` that mapping is keyed by.
It never creates a session, so a route the user has since ended resolves to
nothing, falls back to the captured id, finds no heartbeat, and drops the
watch as before.

Unchanged: busy routes still coalesce their tick, routes whose heartbeat
was genuinely cleared still unregister, and a failing store lookup falls
back to the captured id instead of taking the poller down.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants