Skip to content

fix(gateway): keep pending /update completion notifications until the target platform reconnects - #39091

Merged
teknium1 merged 1 commit into
mainfrom
hermes/hermes-9fa9a6bc
Jun 4, 2026
Merged

teknium1 merged 1 commit into
mainfrom
hermes/hermes-9fa9a6bc

Conversation

@teknium1

@teknium1 teknium1 commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

A /update completion notification now survives a late platform reconnect instead of being silently lost.

Salvage of #38522 by @Frowtek, cherry-picked onto current main with authorship preserved.

Root cause

When hermes update finishes but the target platform's adapter hasn't reconnected yet (common right after the restart the update triggers), _send_update_notification fell through the send block and the finally deleted all completion markers — so the user never learned whether the update succeeded or timed out.

Changes

  • gateway/run.py: _send_update_notification defers (returns False) and preserves the markers when the target adapter isn't connected yet, so a later retry delivers once it reconnects — symmetric with the existing "update still running" defer path. The completion-only watcher fallback now polls until delivery actually succeeds rather than giving up on the first completion check.
  • tests/gateway/test_update_command.py, tests/gateway/test_update_streaming.py: regression coverage (markers preserved while offline; delivered exactly once after reconnect).

Validation

pytest tests/gateway/test_update_command.py tests/gateway/test_update_streaming.py — 49/49 pass.

Closes #38522.

Infographic

gateway-and-auth-hardening

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🔎 Lint report: hermes/hermes-9fa9a6bc 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: 9837 on HEAD, 9837 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 5101 pre-existing issues carried over.

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

@teknium1
teknium1 merged commit b7169f9 into main Jun 4, 2026
20 checks passed
@teknium1
teknium1 deleted the hermes/hermes-9fa9a6bc branch June 4, 2026 13:56
RichardHojunJang added a commit to RichardHojunJang/hermes-agent that referenced this pull request Aug 15, 2026
A stale `.update_pending.json` naming a platform this profile does not run
pins a permanent retry loop for the life of the gateway.

`_send_update_notification` treats "no adapter for the target platform" as a
single case and always defers, preserving the markers so a reconnecting
adapter can still be notified (the behaviour NousResearch#39091 added, which is correct
for a platform that IS enabled and merely still connecting).

But `config.platforms` is pre-seeded with disabled placeholders for the whole
platform catalog, so a marker can name a platform with `enabled=False`. No
adapter will ever appear for it. The watcher then polls every 2s, and each
poll re-reads the marker, logs, and rewrites it -- forever.

Observed on a Slack-only profile carrying an orphan marker that named
telegram: a steady 0.5 lines/sec, ~43k lines/day, which grew to 78% of the
gateway log (9,972 of 12,699 lines) before it was found.

Split the two cases. When the target platform is provably not enabled for
this profile, the target is unreachable by construction: consume the markers
and log once at WARNING with the reason. When it is enabled, keep deferring
exactly as before.

The enabled-check fails open. A missing or unreadable config returns True, so
an unexpected config shape keeps the existing retry behaviour and can never
discard a deliverable notification; only a positive, readable `enabled=False`
ends the retry.

Tests:
- disabled platform: markers consumed, no retry, nothing misdelivered
- enabled but reconnecting: markers preserved (guards the discard path from
  over-reaching)
- unreadable config: fails open to the old deferral

Verified the first test fails on the unpatched tree (`assert False is True`)
and that the other 17 tests in the class still pass there, so it pins this
bug rather than the surrounding behaviour.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants