Skip to content

fix(gateway): keep Telegram topic fork on branch - #58851

Open
Qwinty wants to merge 2 commits into
NousResearch:mainfrom
Qwinty:fix/fork-telegram-topic-binding
Open

Qwinty wants to merge 2 commits into
NousResearch:mainfrom
Qwinty:fix/fork-telegram-topic-binding

Conversation

@Qwinty

@Qwinty Qwinty commented Jul 5, 2026

Copy link
Copy Markdown

Summary

  • keep Telegram DM topic bindings on the new branch after /branch / /fork
  • rename the Telegram DM topic to the explicit branch name when /fork <name> is used
  • add a regression test covering the stale binding rollback and topic rename

Fixes #58850.

Test Plan

  • python -m pytest tests/gateway/test_telegram_topic_mode.py::test_branch_inside_telegram_topic_rewrites_binding_and_renames_topic -q -o 'addopts='
  • python -m pytest tests/gateway/test_telegram_topic_mode.py tests/gateway/test_session_boundary_security_state.py tests/cli/test_branch_command.py -q -o 'addopts='

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P2 Medium — degraded but workaround exists labels Jul 5, 2026

@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 (LGTM)

Small fix: keeps Telegram topic fork on branch. The change is 1 file and addresses a specific Telegram topic branching edge case. Well-scoped and appropriate.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused regression test. The /fork premise is confirmed on current main: gateway/slash_commands.py:3882 switches the active session, while gateway/run.py:10976-11008 subsequently applies the still-persisted topic binding and can switch the lane back to its parent. The proposed post-switch sync matches the existing helper at gateway/run.py:3672-3696, and placing it before the rename is correct because the rename helper verifies that binding at gateway/run.py:13919-13927.

Problems

  • The equivalent /resume path remains vulnerable: gateway/slash_commands.py:3677-3679 switches a session without updating a Telegram topic binding, so the inbound binding lookup at gateway/run.py:10976-11008 can restore the prior topic session.

Suggested changes

  • Synchronize the binding after successful /resume in a Telegram topic lane and add a regression test for that route.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 15, 2026
@Qwinty
Qwinty force-pushed the fix/fork-telegram-topic-binding branch from 118ee76 to 703ac27 Compare July 16, 2026 07:23
@Qwinty

Qwinty commented Jul 16, 2026

Copy link
Copy Markdown
Author

Addressed in 703ac27c3 after rebasing onto current upstream/main. A successful /resume now refreshes the Telegram topic binding to the selected session before the next inbound binding lookup can restore the prior lane. Added a persisted-row regression for /resume, and updated the /fork regression to exercise the current async session-store boundary. Local validation: 47 passed for tests/gateway/test_telegram_topic_mode.py, 52 passed for tests/gateway/test_resume_command.py, ruff passed, and git diff --check passed.

@Qwinty
Qwinty force-pushed the fix/fork-telegram-topic-binding branch from 703ac27 to a803d8e Compare July 19, 2026 13:30
@Qwinty

Qwinty commented Jul 19, 2026

Copy link
Copy Markdown
Author

Rebased onto current upstream/main.

Conflict in /resume was resolved by keeping main's newer conversation-scope clear path and re-applying the Telegram topic-binding sync after switch_session (without restoring the old _clear_session_boundary_security_state call that main replaced).

Local verification:

python -m pytest -q -o 'addopts=' tests/gateway/test_telegram_topic_mode.py
# 47 passed

@Qwinty

Qwinty commented Aug 21, 2026

Copy link
Copy Markdown
Author

Maintainer review requested: this remains a narrow, mergeable fix with green CI. Prior review confirmed the /fork premise on current main and the branch preserves the Telegram topic binding after switching to the child session.

This branch has not been deployed

No deployments
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 sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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.

Telegram DM topic /fork routes back to parent session

4 participants