Skip to content

fix(gateway): don't hijack brand-new Telegram DM topics into previous session#29287

Closed
fabiosiqueira wants to merge 1 commit into
NousResearch:mainfrom
fabiosiqueira:fix/telegram-new-topic-hijack
Closed

fix(gateway): don't hijack brand-new Telegram DM topics into previous session#29287
fabiosiqueira wants to merge 1 commit into
NousResearch:mainfrom
fabiosiqueira:fix/telegram-new-topic-hijack

Conversation

@fabiosiqueira

Copy link
Copy Markdown
Contributor

Regression introduced by #26609

Commit ede47a54b added _recover_telegram_topic_thread_id to pin inbound DM topic routing to the user's most-recent binding when the inbound thread_id is either lobby/General or unknown to existing bindings.

The "unknown → recover" branch is wrong: when a user creates a new Telegram DM topic, the inbound thread_id is real and non-lobby, but it has no binding yet because _record_telegram_topic_binding runs at line ~7876 — after recovery has already decided to redirect. The result: every message in the new topic is silently routed into the previously-active topic's session, and the new topic is never bound.

Live evidence from gateway logs:

telegram topic recovery: chat=1232992028 user=1232992028 '27453' -> 27374
telegram topic recovery: chat=1232992028 user=1232992028 '27453' -> 27374
telegram topic recovery: chat=1232992028 user=1232992028 '27453' -> 27374

27453 is the freshly-created topic id; 27374 is the previous session's topic. Every message keeps being hijacked because the binding for 27453 is never written.

The fix

The two cases that warrant recovery are:

  1. thread_id is None / empty — _build_message_event stripped it ([Bug]: Telegram DM sends fail with 'Message thread not found' — spurious thread_id from reply chains #3206).
  2. thread_id is a Telegram General/lobby magic id.

Both are lobby cases. A non-lobby thread_id always belongs to the topic the user intends — whether it is already bound or brand new. The fix collapses the recovery guard to is_lobby only and removes the known set lookup entirely.

Trade-off: a reply to a message in a deleted topic delivers a non-lobby but truly stale thread_id. Previously that would have been recovered; now it becomes an orphan message with no binding — consistent with what Telegram itself shows the user when the topic is gone.

Test changes

  • test_recover_rewrites_unknown_thread_id_to_most_recent → renamed to test_recover_returns_none_for_unknown_thread_id and body updated to assert None (no hijack).
  • Added test_recover_returns_none_for_brand_new_topic: bindings exist for topic 12345, inbound thread_id="99999" (non-lobby, unbound). Asserts None.
  • All previously-passing tests (test_recover_returns_none_for_known_topic, test_recover_rewrites_lobby_thread_id_to_most_recent, test_recover_returns_none_when_topic_mode_disabled, test_recover_returns_none_when_no_bindings_yet) remain unchanged and pass.

@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 labels May 20, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #28605 — both fix the same regression from #26609 where _recover_telegram_topic_thread_id hijacks brand-new DM topic lanes by rewriting unknown non-lobby thread_ids to the most-recent binding. Both PRs narrow recovery to lobby-shaped inputs only.

@fabiosiqueira

Copy link
Copy Markdown
Contributor Author

The 2 test failures here are pre-existing on main (latest main run 26154598271 fails on the exact same 2 tests at commit 42c4288, before this PR was opened):

  • tests/plugins/web/test_web_search_provider_plugins.py::TestBundledPluginsRegister::test_all_seven_plugins_present_in_registry — the bundled registry now has 8 plugins (xai was added) but the test still expects 7.
  • tests/hermes_cli/test_update_hangup_protection.py::TestInstallHangupProtection::test_wraps_stdout_and_stderr_with_mirrorsys.stdout isolation leak from another test.

Neither test touches gateway/run.py or anything Telegram-related. The new + updated tests in tests/gateway/test_telegram_topic_mode.py all pass (24490 passed, 2 failed, 146 skipped).

… session

_recover_telegram_topic_thread_id treated any unknown thread_id as a
stale cross-topic reply and rewrote it to the user's most-recent
binding. That also matches a freshly-created topic, whose binding is
only recorded after recovery decides not to rewrite — so every message
in the new topic was silently merged into the previously-active
topic's session, and the new topic was never bound.

Limit recovery to the lobby/General case (the original PR NousResearch#3206 strip
bug). Replies to deleted-topic messages become a rare orphan case,
matching what Telegram itself shows the user.

Updates the existing unknown-thread_id test to assert the new
no-hijack behavior; adds a brand-new-topic regression test.
@teknium1

Copy link
Copy Markdown
Contributor

Superseded by #31444 (#31444). Your brand-new-topic regression test pattern was adopted as the second test in the merged PR, with you credited as a co-author on the follow-up commit. Thanks for the strong test coverage on this one. Closes #31086.

@teknium1 teknium1 closed this May 24, 2026
@fabiosiqueira fabiosiqueira deleted the fix/telegram-new-topic-hijack branch May 25, 2026 03:52
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 platform/telegram Telegram bot adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants