Skip to content

fix(telegram): validate reply message IDs - #72937

Open
ooiuuii wants to merge 1 commit into
NousResearch:mainfrom
ooiuuii:fix/telegram-reply-id-validation
Open

fix(telegram): validate reply message IDs#72937
ooiuuii wants to merge 1 commit into
NousResearch:mainfrom
ooiuuii:fix/telegram-reply-id-validation

Conversation

@ooiuuii

@ooiuuii ooiuuii commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Validates Telegram reply-message IDs before any outbound routing path uses
them.

The adapter now accepts only positive 32-bit message IDs, falls back to a
valid DM-topic metadata anchor when an explicit value is malformed, and
refuses an unroutable private-topic send instead of raising or sending outside
the requested lane.

The behavior is aligned across legacy text, rich text, control prompts, voice,
images, documents, and other attachment paths. Existing
reply_to_mode: off text/rich behavior remains intact.

Related Issue

Fixes #72934

Type of Change

  • Bug fix
  • Tests

Changes Made

  • plugins/platforms/telegram/adapter.py:
    • centralize reply-ID normalization;
    • enforce Telegram's 1..2147483647 message-ID range;
    • use a valid metadata anchor when an explicit value is invalid;
    • fail closed and non-retryably when a DM-topic send has no valid route;
    • keep rich, legacy, control, and media routing consistent.
  • tests/gateway/test_telegram_reply_mode.py: cover malformed, oversized,
    boundary-value, metadata-fallback, control, document, and legacy sends.
  • tests/gateway/test_telegram_rich_messages.py: cover metadata fallback,
    non-retryable missing anchors, and reply_to_mode: off on the rich path.

How to Test

Run each routing group in an isolated pytest process:

python -m pytest -q tests/gateway/test_telegram_reply_mode.py
python -m pytest -q tests/gateway/test_telegram_thread_fallback.py
python -m pytest -q tests/gateway/test_telegram_rich_messages.py tests/gateway/test_telegram_documents.py
python -m ruff check plugins/platforms/telegram/adapter.py tests/gateway/test_telegram_reply_mode.py tests/gateway/test_telegram_rich_messages.py
git diff --check origin/main...HEAD

Validation on rebased dbc18c6d:

  • Reply-mode tests: 65 passed.
  • Thread-routing tests: 51 passed.
  • Rich/document tests: 119 passed.
  • Total focused tests: 235 passed.
  • Ruff: passed.
  • git diff --check: passed.
  • Structured Codex autoreview: clean, no actionable findings.

Checklist

Code

  • I've read the Contributing Guide.
  • My commit message follows Conventional Commits.
  • I searched open and closed issues/PRs for duplicates.
  • This PR contains only Telegram reply-ID routing changes and tests.
  • I've run the full pytest tests/ -q suite; focused tests are listed
    above.
  • I've added regression tests.
  • Tested on Windows 11 with Python 3.11.

Documentation & Housekeeping

  • Documentation update: N/A; no public configuration changed.
  • Config example update: N/A.
  • Architecture/workflow docs: N/A.
  • Cross-platform impact considered; parsing is platform-independent.
  • Tool schema update: N/A.

Screenshots / Logs

Before:

ValueError: invalid literal for int() with base 10: 'session-meta-id'

After:

invalid explicit ID + valid metadata anchor -> metadata anchor used
invalid DM-topic anchor without a direct topic -> non-retryable refusal
235 focused tests passed

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins platform/telegram Telegram bot adapter P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 27, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused routing fix. The premise is verified on current main: unguarded reply-ID conversions remain in plugins/platforms/telegram/adapter.py:1117, plugins/platforms/telegram/adapter.py:1754, and plugins/platforms/telegram/adapter.py:4423. The shared validation in 63f1b4fa5310 (plugins/platforms/telegram/adapter.py:1113-1129) covers malformed and out-of-range values, while the existing metadata fallback and fail-closed routing paths are retained.

No blocking correctness or design-fit issue was identified in the PR diff. This is an automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

One PR addresses #72934. #72937 replaces duplicated, unguarded Telegram reply-ID conversions with bounded normalization, metadata-anchor fallback, and fail-closed private DM-topic routing across text, rich, control, and attachment paths.

Related pull requests

  • fix(telegram): validate reply message IDs #72937 best fix — (+261/-23) — keep open with a salvage path: The diff centralizes positive 32-bit reply-ID validation, falls back from malformed explicit IDs to valid metadata anchors, preserves reply_to_mode: off behavior, and adds focused cross-path regression tests. Consistent with the automated keep-open review on fix(telegram): validate reply message IDs #72937, no blocking correctness or design-fit issue is evident; the salvageable unit is the shared validator plus the fallback and fail-closed routing changes with their tests.

Suggested consolidation

Keep #72937 open with a salvage path: retain the shared reply-ID normalizer, metadata-anchor fallback, fail-closed DM-topic routing, and associated regression coverage as one focused fix for #72934. There are no competing or duplicate PRs to close.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I72934(["issue #72934 (open)"])
    P72937["PR #72937 (open)"]
    P72937 -->|best fix| I72934
    class I72934 open
    class P72937 open
    class P72937 best
    class P72937 target
    click I72934 "https://github.com/NousResearch/hermes-agent/issues/72934"
    click P72937 "https://github.com/NousResearch/hermes-agent/pull/72937"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 16 kB of PR diffs, 6 kB of issue/PR text, 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@andrexibiza

Copy link
Copy Markdown
Contributor

Composition heads-up from the adapter decomposition campaign (epic #78791): PR #78949 (slice A1) moves the DM-topic cluster — including _metadata_reply_to_message_id and _reply_to_message_id_for_send, the two methods this PR edits — into plugins/platforms/telegram/telegram_dm_topics.py (class TelegramDmTopicMixin), byte-identical bodies, zero behavior change. Verified: A1 does not carry your reply-ID validation work; it stays yours.

Merge order so nothing is lost: merge #78949 first, then rebase this PR onto main — your hunks then apply to the mixin file cleanly (your base's parents are ancestors of A1's base, verified). Your fix is a good complement; the extraction deliberately preserves your seam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid Telegram reply IDs can crash or misroute DM topic sends

5 participants