Skip to content

fix(telegram): reply to latest queued follow-up - #66027

Open
Hilo-Hilo wants to merge 4 commits into
NousResearch:mainfrom
Hilo-Hilo:fix/telegram-queued-reply-target
Open

fix(telegram): reply to latest queued follow-up#66027
Hilo-Hilo wants to merge 4 commits into
NousResearch:mainfrom
Hilo-Hilo:fix/telegram-queued-reply-target

Conversation

@Hilo-Hilo

@Hilo-Hilo Hilo-Hilo commented Jul 17, 2026

Copy link
Copy Markdown

Bug Description

When multiple Telegram follow-ups arrive while a session is busy—or are coalesced by Telegram ingress batching—Hermes can combine their text but visibly reply to an older message or inject stale quoted-message context into the model.

Root Cause

Merged events appended newer content while retaining parts of the first event's trigger tuple. The production queue-mode debounce path, Telegram text batching, photo batching, and album/media-group batching could bypass or partially apply the original metadata refresh.

That produced mixed events such as a newest message_id paired with an older reply_to_text, author, raw update, timestamp, or trust provenance. An internal event could also leave a merged user event marked internal.

Fix

  • Centralize merged-event trigger/reply/provenance refresh.
  • Apply it to direct text/media merges, queue-mode debounce and flush, Telegram ingress text batching, photo batching, and media groups.
  • Advance message ID, reply ID/text/author/ownership, raw message, platform update ID, timestamp, channel context, and metadata coherently to the newest event.
  • Merge metadata with newest values winning.
  • Keep a merged event internal=True only when every constituent event was internal, preventing privilege escalation.
  • Preserve the last concrete platform message ID for intentionally ID-less synthetic inputs while replacing stale quoted-message context.

Test Plan

  • Regressions for newest-message anchoring, clearing stale quotes, and ID-less synthetic inputs.
  • End-to-end queue debounce → flush regression verifies coherent newest reply metadata.
  • Telegram ingress text-batching regression verifies newest trigger/reply/provenance metadata.
  • Photo and album/media-group batching regression verifies newest reply targets.
  • Mixed internal/user merge regression verifies latest raw/update/channel metadata and no privilege escalation.
  • Rebased onto current origin/main (7cb2d2cd4).
  • Focused queue/debounce/batching/pending surface: 54 passed.
  • Combined current-upstream gateway suite with PR fix(telegram): prevent duplicate finals after failed edit #66026: 9462 passed, 13 skipped, 15 failed.
  • Untouched current origin/main under the same environment: 9449 passed, 13 skipped, 15 failed.
  • The 15 failing node IDs are identical between baseline and combined trees; the combined tree adds 13 passing regressions and introduces no new failures.
  • Fresh isolated adversarial regressions: 5 passed.
  • git diff --check and Ruff pass.

Risk Assessment

Low-to-moderate. Accumulated content and media behavior are unchanged; only the identity, reply context, provenance, and trust state of an already-merged event are made coherent with its newest constituent.

@Hilo-Hilo
Hilo-Hilo marked this pull request as ready for review July 17, 2026 01:27
@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/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 labels Jul 17, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #43787 (Telegram batching) and #64752 (queued-turn return anchoring). This fixes the shared pending-event merge path, so it is related rather than a duplicate.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Comment

Small fix (64 lines). Telegram queued reply target fix.

Checked diff — no security concerns, no hardcoded secrets.

Looks good. No blocking issues.


Reviewed by Hermes Agent

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary (Re-review)

Verdict: LGTM / Comment

Fix ensures Telegram bot replies to the latest queued follow-up rather than an earlier one. Prior COMMENT review confirmed. No issues found.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the queued-event and Telegram ingress paths. The premise is confirmed on current main: gateway/platforms/base.py:2157-2164 appends merged text without updating event identity or reply context, while gateway/run.py:11485-11499 injects that reply context into the model prompt. The proposed helper updates the relevant trigger tuple and preserves the existing message ID for ID-less synthetic inputs; the Telegram batching call sites and regressions cover the stated scope.

Suggested changes

  • Consider a follow-up audit of other native text batchers. plugins/platforms/discord/adapter.py:6877-6882, plugins/platforms/matrix/adapter.py:3588-3595, and plugins/platforms/whatsapp/adapter.py:1297-1303 use the same append-without-metadata-refresh pattern. This PR is correctly scoped to Telegram, but those paths may need the same treatment if they rely on MessageEvent.message_id or reply context.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 18, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Four PRs were reviewed: #46459 and #63298 directly address #45560, while #31496 fixes a separate three-sender delivery-loss path and #66027 fixes stale Telegram trigger/reply metadata after merging. #46459 adds visible boundaries while still collapsing canonical turns; #63298 preserves FIFO, history, and SessionDB boundaries and merges only the provider copy.

Related pull requests

Duplicates

#46459 and #63298 overlap on #45560 and explicit provider-visible boundary markers; #46459 is the narrower implementation and can be closed as a duplicate superseded by #63298. #31496 and #66027 address separate gateway defects and are not duplicates of them.

Suggested consolidation

Keep #63298 open with the salvage path centered on its FIFO, canonical-history/SessionDB boundary preservation, stable source IDs, and wire-only alternation repair; require the current head to retain the restored steer contract. Close #46459 as duplicate of #63298 despite its keep-open review because its diff still collapses canonical turns and omits the TUI queue root cause; keep #66027 open as a separate Telegram metadata fix, and require author action on #31496 to rebase or split out a separate-event FIFO solution that resolves the contributor's source-attribution blocker.

Cross-PR triage: Reviewed 4 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 186 kB of PR diffs, 10 kB of issue/PR text, 12 kB of discussion (16 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@alt-glitch alt-glitch added comp/plugins Plugin system and bundled plugins needs-decision Awaiting maintainer decision before any implementation and removed sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data comp/plugins Plugin system and bundled plugins labels Aug 4, 2026
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 needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit 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.

5 participants