Skip to content

fix(gateway): admit completion wakes whose source differs only by message_id - #138967

Closed
sharpnotebot wants to merge 1 commit into
NousResearch:mainfrom
sharpnotebot:fix/automation-wake-message-id
Closed

sharpnotebot wants to merge 1 commit into
NousResearch:mainfrom
sharpnotebot:fix/automation-wake-message-id

Conversation

@sharpnotebot

Copy link
Copy Markdown

What does this PR do?

A background-process completion wake is refused with admission_conflict on every attempt, so it never reaches the session and is requeued unchanged (#138940 on Discord, #134889 on Telegram).

snapshot_automation restores the session's last committed native envelope and compared its source to the synthetic event's source exactly. The synthetic completion deliberately clears source.message_id (#52694, #123097) so a finished job does not reply-anchor to the message that started it, while Discord and Telegram put the human message id on the committed source. SessionSource.to_dict() writes message_id only when it is truthy, so the two sides could never be equal.

This PR compares the routing/provenance identity of both sources with only message_id excluded, keeps every other source field and the adapter identity check, and drops message_id from the persisted automation envelope so the wake cannot anchor to the stale human message (it restores as message_id=None). The key is omitted rather than stored as null because that is the canonical SessionSource.to_dict() format (it never writes a null anchor), so the stored source looks exactly like one built from a source with no message_id, and no new null key enters the hashed payload.

Fingerprint compatibility is supported only by a digest probe, not by an integration test: hashing a payload whose source omits message_id gives the same digest as one whose message_id was popped, and a different digest from one carrying message_id: null. Retrying, across versions, an admission recorded before this change is not tested here.

#138947 is complementary rather than a duplicate: it bounds the retry loop for a refused admission (stops the log flood), while this PR removes the cause of the refusal so the wake is actually delivered. Both can land.

Background:

Related Issue

Fixes #138940

Type of Change

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

Changes Made

  • gateway/session_automation.py — snapshot_automation compares sources through _route_identity() (to_dict() without message_id) on both sides; the adapter identity check is unchanged; the stored automation envelope source no longer carries message_id.
  • tests/gateway/test_automation_notification_category.py — native Discord regressions in which the committed human source carries message_id (the existing roundtrip test put the id only on the MessageEvent, so it never exercised this):
    • the synthetic wake (source message_id=None) admits, the stored envelope source has no message_id key, the restored source's message_id is None, and a retry returns the same admission (one automation admission);
    • a source that differs in user_name, user_id or thread_id still raises admission_conflict;
    • a different adapter instance still raises admission_conflict.

How to Test

  1. scripts/run_tests.sh tests/gateway/test_automation_notification_category.py -k "message_id or source_drift or another_adapter" -v
  2. scripts/run_tests.sh tests/gateway/test_automation_notification_category.py tests/gateway/test_producer_owner_continuity.py tests/gateway/test_plugin_message_injection.py tests/gateway/test_heartbeat_inflight_owner_scan.py tests/gateway/test_bot_mailbox_async.py tests/gateway/test_webhook_durable_destination.py
  3. ruff check gateway/session_automation.py tests/gateway/test_automation_notification_category.py

Evidence

BEFORE (new tests on main 450ad5055e, product change stashed) — exit 1:

FAILED tests/gateway/test_automation_notification_category.py::test_native_completion_wake_admits_when_source_differs_only_by_message_id
E           hermes_state_runtime.RuntimeStoreError: admission_conflict
gateway/session_automation.py:136: RuntimeStoreError
================== 1 failed, 4 passed, 10 deselected in 5.93s ==================

(The BEFORE run predates tightening the stored-source check from .get('message_id') is None to 'message_id' not in source; on main the test fails earlier, at admission, so the tightened assertion was only run after the fix.)

The runs below used the same hermetic environment as scripts/run_tests.sh's final exec (env -i … python scripts/run_tests_parallel.py … -p no:cacheprovider, PYTHONDONTWRITEBYTECODE=1), skipping only its compileall pre-step, which writes .pyc files for every tracked file.

AFTER (same selection) — exit 0:

=== Summary: 1 files, 5 tests passed, 0 failed (100% complete) in 6.9s (4 workers) ===

Related gateway files (six files above) — exit 0:

=== Summary: 6 files, 47 tests passed, 0 failed (100% complete) in 15.3s (4 workers) ===

An earlier run of the same six files reported tests/gateway/test_bot_mailbox_async.py::test_drain_defers_retry_without_publishing_interim_failure as flaky (failed once, passed on retry, assert 'settled' == 'claimed'). That test drives the local bot mailbox, which returns through snapshot_local_automation before the changed code; it passed in three isolated reruns and in the final run above (no flaky file reported).

Lint — exit 0:

$ ruff check --no-cache gateway/session_automation.py tests/gateway/test_automation_notification_category.py
All checks passed!

Checklist

Code

Documentation & Housekeeping

  • Documentation — N/A
  • cli-config.yaml.example — N/A
  • CONTRIBUTING.md / AGENTS.md — N/A
  • Cross-platform impact — N/A (pure dict comparison)
  • Tool descriptions/schemas — N/A

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/discord Discord bot adapter platform/telegram Telegram bot adapter area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Oct 15, 2026
@dplush

dplush commented Oct 15, 2026

Copy link
Copy Markdown

Independent synthetic validation on 01e86ff: the source-message_id regression passes. Combining this head with only the base.py change from #138986 (82a4d3d), 9 additional tests pass through real temporary SQLite/native admission and restore for Discord and Telegram. They cover no reply anchor, explicit override, idempotent retry, and rejection of chat/user/thread/profile/name and connector/home provenance drift. On this PR alone, the two positive roundtrips admit successfully but fail at the reply-anchor assertion, confirming the complementary fix is needed. No live adapter sends, production changes, or full-suite claim.

kshitijk4poor added a commit that referenced this pull request Oct 15, 2026
… no-anchor branch

The code-health ratchet caps gateway/platforms/base.py at its size on main
(4914 lines); the internal-wake guard from #138967's companion fix added
five. Both branches return None before any platform-specific anchor, so
they share one condition. Behaviour is unchanged: the guard still runs
after reply_anchor_override and before the Telegram DM-topic branch.
@kshitijk4poor

Copy link
Copy Markdown

Thanks @sharpnotebot. This commit and your companion #138986 are carried into #139026 with your authorship (d9670a2eff, 4ef0d04593), together with @KoNit-K's retry cap from #138947 and a bounded retry with a plain-notice fallback. Closing in favour of it.

One change on top of #138986: its guard is folded into the existing Slack no-anchor branch (same order, after reply_anchor_override). gateway/platforms/base.py is at its code-health line cap and can't grow.

#139026 is armed. I'll post the merge SHA here once it lands, and reopen this if it doesn't.

@kshitijk4poor

Copy link
Copy Markdown

Thanks @dplush. Your finding (this head alone admits, but the reply-anchor assertion fails without #138986) is why both commits ship together in #139026.

kshitijk4poor added a commit that referenced this pull request Oct 15, 2026
… no-anchor branch

The code-health ratchet caps gateway/platforms/base.py at its size on main
(4914 lines); the internal-wake guard from #138967's companion fix added
five. Both branches return None before any platform-specific anchor, so
they share one condition. Behaviour is unchanged: the guard still runs
after reply_anchor_override and before the Telegram DM-topic branch.
@kshitijk4poor

Copy link
Copy Markdown

Landed on main via #139026 (merge head 2bc1edbc5e). Thanks again.

foXaCe pushed a commit to foXaCe/hermes-agent that referenced this pull request Oct 16, 2026
foXaCe pushed a commit to foXaCe/hermes-agent that referenced this pull request Oct 16, 2026
… no-anchor branch

The code-health ratchet caps gateway/platforms/base.py at its size on main
(4914 lines); the internal-wake guard from NousResearch#138967's companion fix added
five. Both branches return None before any platform-specific anchor, so
they share one condition. Behaviour is unchanged: the guard still runs
after reply_anchor_override and before the Telegram DM-topic branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/discord Discord bot adapter platform/telegram Telegram bot adapter 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.

[Bug]: Background process completion notifications fail delivery to Discord and spin in infinite retry injection loop

4 participants